Skip to content

Commit

Permalink
fix: return valid MsgId from receive_imf() when the message is replaced
Browse files Browse the repository at this point in the history
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
  • Loading branch information
link2xt committed Jul 11, 2023
1 parent a464cbd commit db941cc
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 6 deletions.
14 changes: 10 additions & 4 deletions src/receive_imf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down Expand Up @@ -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?;
Expand All @@ -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
Expand Down
6 changes: 4 additions & 2 deletions src/webxdc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1177,15 +1177,17 @@ 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(),
false,
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);
Expand Down

0 comments on commit db941cc

Please sign in to comment.