From db941ccf881c045bad1c460b942beb21442c4ee2 Mon Sep 17 00:00:00 2001
From: link2xt <link2xt@testrun.org>
Date: Tue, 11 Jul 2023 03:33:07 +0000
Subject: [PATCH] fix: return valid MsgId from receive_imf() when the message
 is replaced

receive_imf() calls add_parts()
which INSERTs or UPDATEs the message using UPSERT [1].
It then uses last_insert_rowid() to get
the ID of the inserted message.
However, it is incorrect to use last_insert_rowid()
if an UPDATE was executed instead of INSERT.
The solution is to use `RETURNING id` clause
to make the UPSERT statement return message ID in any case [2].

The fix is tested in test_webxdc_update_for_not_downloaded_instance()
and with a debug_assert!.

[1] https://www.sqlite.org/lang_UPSERT.html
[2] https://sqlite.org/forum/forumpost/9ce3bc1c4a85c15f
---
 src/receive_imf.rs | 14 ++++++++++----
 src/webxdc.rs      |  6 ++++--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/receive_imf.rs b/src/receive_imf.rs
index 6db785650..e9b885338 100644
--- a/src/receive_imf.rs
+++ b/src/receive_imf.rs
@@ -1178,8 +1178,9 @@ SET rfc724_mid=excluded.rfc724_mid, chat_id=excluded.chat_id,
     mime_compressed=excluded.mime_compressed, mime_in_reply_to=excluded.mime_in_reply_to,
     mime_references=excluded.mime_references, mime_modified=excluded.mime_modified, error=excluded.error, ephemeral_timer=excluded.ephemeral_timer,
     ephemeral_timestamp=excluded.ephemeral_timestamp, download_state=excluded.download_state, hop_info=excluded.hop_info
+RETURNING id
 "#)?;
-                stmt.execute(params![
+                let row_id: MsgId = stmt.query_row(params![
                     replace_msg_id,
                     rfc724_mid,
                     if trash { DC_CHAT_ID_TRASH } else { chat_id },
@@ -1218,8 +1219,12 @@ SET rfc724_mid=excluded.rfc724_mid, chat_id=excluded.chat_id,
                         DownloadState::Done
                     },
                     mime_parser.hop_info
-                ])?;
-                let row_id = conn.last_insert_rowid();
+                ],
+                |row| {
+                    let msg_id: MsgId = row.get(0)?;
+                    Ok(msg_id)
+                }
+                )?;
                 Ok(row_id)
             })
             .await?;
@@ -1228,7 +1233,8 @@ SET rfc724_mid=excluded.rfc724_mid, chat_id=excluded.chat_id,
         // afterwards insert additional parts.
         replace_msg_id = None;
 
-        created_db_entries.push(MsgId::new(u32::try_from(row_id)?));
+        debug_assert!(!row_id.is_special());
+        created_db_entries.push(row_id);
     }
 
     // check all parts whether they contain a new logging webxdc
diff --git a/src/webxdc.rs b/src/webxdc.rs
index 0b6dbc1ca..1f3215365 100644
--- a/src/webxdc.rs
+++ b/src/webxdc.rs
@@ -1177,7 +1177,7 @@ mod tests {
         assert_eq!(bob_instance.download_state, DownloadState::Available);
 
         // Bob downloads instance, updates should be assigned correctly
-        receive_imf_inner(
+        let received_msg = receive_imf_inner(
             &bob,
             &alice_instance.rfc724_mid,
             sent1.payload().as_bytes(),
@@ -1185,7 +1185,9 @@ mod tests {
             None,
             false,
         )
-        .await?;
+        .await?
+        .unwrap();
+        assert_eq!(*received_msg.msg_ids.get(0).unwrap(), bob_instance.id);
         let bob_instance = bob.get_last_msg().await;
         assert_eq!(bob_instance.viewtype, Viewtype::Webxdc);
         assert_eq!(bob_instance.download_state, DownloadState::Done);