Skip to content

Commit

Permalink
refactor: Don't update message state to OutMdnRcvd anymore
Browse files Browse the repository at this point in the history 
This state can be computed from the `msgs_mdns` table without significant overhead as we have an
index by msg_id there.
  • Loading branch information
@iequidoo
iequidoo committed Jul 31, 2024
1 parent 490f41c commit 49a26e1
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 40 deletions.
44 changes: 36 additions & 8 deletions src/message.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,",
Expand All @@ -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",
" WHERE m.id=? AND chat_id!=3;"
" FROM msgs m",
" 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()) {
Expand Down Expand Up @@ -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()),
Expand Down Expand Up @@ -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,
}

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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=''",
Expand Down Expand Up @@ -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;

Expand Down
58 changes: 26 additions & 32 deletions src/mimeparser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
self, get_vcard_summary, set_msg_failed, update_msg_state, Message, MessageState, MsgId,
Viewtype,
};
use crate::message::{self, get_vcard_summary, set_msg_failed, Message, MsgId, Viewtype};
use crate::param::{Param, Params};
use crate::peerstate::Peerstate;
use crate::simplify::{simplify, SimplifiedText};
Expand Down Expand Up @@ -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",
" FROM msgs m LEFT JOIN chats c ON m.chat_id=c.id",
" mdns.contact_id AS mdn_contact",
" 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")?;
Ok((msg_id, chat_id, msg_state))
let mdn_contact: Option<ContactId> = row.get("mdn_contact")?;
Ok((
msg_id,
chat_id,
mdn_contact.is_some(),
mdn_contact == Some(from_id),
))
},
)
.await?
Expand All @@ -2186,28 +2191,17 @@ async fn handle_mdn(
return Ok(());
};

if !context
if is_dup {
return Ok(());
}
context
.sql
.exists(
"SELECT COUNT(*) FROM msgs_mdns WHERE msg_id=? AND contact_id=?",
(msg_id, from_id),
.execute(
"INSERT INTO msgs_mdns (msg_id, contact_id, timestamp_sent) VALUES (?, ?, ?)",
(msg_id, from_id, timestamp_sent),
)
.await?
{
context
.sql
.execute(
"INSERT INTO msgs_mdns (msg_id, contact_id, timestamp_sent) VALUES (?, ?, ?)",
(msg_id, from_id, timestamp_sent),
)
.await?;
}

if msg_state == MessageState::OutPreparing
|| msg_state == MessageState::OutPending
|| msg_state == MessageState::OutDelivered
{
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);
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 49a26e1

Please sign in to comment.