From 2580a28dd5ce8f4103a3a8194fe2f490b5b87e16 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Thu, 11 Jul 2024 16:15:04 -0300 Subject: [PATCH] fix: Emit MsgsChanged if the number of unnoticed archived chats could decrease (#5768) Emit `MsgsChanged` for DC_CHAT_ID_ARCHIVED_LINK if the number of unnoticed archived chats could decrease. In general we don't want to make an extra db query to know if a noticied chat is archived. Emitting events should be cheap, better to allow false-positive `MsgsChanged` events. --- src/chat.rs | 69 ++++++++++++++++++++++++++++++++------------------ src/imap.rs | 3 +++ src/message.rs | 53 +++++++++++++++++++++++--------------- 3 files changed, 80 insertions(+), 45 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 368de5fa10..79ec4b0e30 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -3267,35 +3267,25 @@ pub async fn marknoticed_chat(context: &Context, chat_id: ChatId) -> Result<()> context.emit_event(EventType::MsgsNoticed(chat_id_in_archive)); chatlist_events::emit_chatlist_item_changed(context, chat_id_in_archive); } - chatlist_events::emit_chatlist_item_changed(context, DC_CHAT_ID_ARCHIVED_LINK); - } else { - let exists = context - .sql - .exists( - "SELECT COUNT(*) FROM msgs WHERE state=? AND hidden=0 AND chat_id=?;", - (MessageState::InFresh, chat_id), - ) - .await?; - if !exists { - return Ok(()); - } - - context - .sql - .execute( - "UPDATE msgs - SET state=? - WHERE state=? - AND hidden=0 - AND chat_id=?;", - (MessageState::InNoticed, MessageState::InFresh, chat_id), - ) - .await?; + } else if context + .sql + .execute( + "UPDATE msgs + SET state=? + WHERE state=? + AND hidden=0 + AND chat_id=?;", + (MessageState::InNoticed, MessageState::InFresh, chat_id), + ) + .await? + == 0 + { + return Ok(()); } context.emit_event(EventType::MsgsNoticed(chat_id)); chatlist_events::emit_chatlist_item_changed(context, chat_id); - + context.on_archived_chats_maybe_noticed(); Ok(()) } @@ -3358,6 +3348,7 @@ pub(crate) async fn mark_old_messages_as_noticed( context, "Marking chats as noticed because there are newer outgoing messages: {changed_chats:?}." ); + context.on_archived_chats_maybe_noticed(); } for c in changed_chats { @@ -4664,6 +4655,14 @@ impl Context { SyncAction::SetContacts(addrs) => set_contacts_by_addrs(self, chat_id, addrs).await, } } + + /// Emits the appropriate `MsgsChanged` event. Should be called if the number of unnoticed + /// archived chats could decrease. In general we don't want to make an extra db query to know if + /// a noticied chat is archived. Emitting events should be cheap, a false-positive `MsgsChanged` + /// is ok. + pub(crate) fn on_archived_chats_maybe_noticed(&self) { + self.emit_msgs_changed(DC_CHAT_ID_ARCHIVED_LINK, MsgId::new(0)); + } } #[cfg(test)] @@ -5835,7 +5834,27 @@ mod tests { assert_eq!(DC_CHAT_ID_ARCHIVED_LINK.get_fresh_msg_cnt(&t).await?, 2); // mark one of the archived+muted chats as noticed: check that the archive-link counter is changed as well + t.evtracker.clear_events(); marknoticed_chat(&t, claire_chat_id).await?; + let ev = t + .evtracker + .get_matching(|ev| { + matches!( + ev, + EventType::MsgsChanged { + chat_id: DC_CHAT_ID_ARCHIVED_LINK, + .. + } + ) + }) + .await; + assert_eq!( + ev, + EventType::MsgsChanged { + chat_id: DC_CHAT_ID_ARCHIVED_LINK, + msg_id: MsgId::new(0), + } + ); assert_eq!(bob_chat_id.get_fresh_msg_cnt(&t).await?, 2); assert_eq!(claire_chat_id.get_fresh_msg_cnt(&t).await?, 0); assert_eq!(DC_CHAT_ID_ARCHIVED_LINK.get_fresh_msg_cnt(&t).await?, 1); diff --git a/src/imap.rs b/src/imap.rs index 8c2dcb1501..c48b2bc1b3 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -1201,6 +1201,9 @@ impl Session { set_modseq(context, folder, highest_modseq) .await .with_context(|| format!("failed to set MODSEQ for folder {folder}"))?; + if !updated_chat_ids.is_empty() { + context.on_archived_chats_maybe_noticed(); + } for updated_chat_id in updated_chat_ids { context.emit_event(EventType::MsgsNoticed(updated_chat_id)); chatlist_events::emit_chatlist_item_changed(context, updated_chat_id); diff --git a/src/message.rs b/src/message.rs index f97a3a925a..bad540fbc1 100644 --- a/src/message.rs +++ b/src/message.rs @@ -11,7 +11,7 @@ use serde::{Deserialize, Serialize}; use tokio::{fs, io}; use crate::blob::BlobObject; -use crate::chat::{Chat, ChatId, ChatIdBlocked}; +use crate::chat::{Chat, ChatId, ChatIdBlocked, ChatVisibility}; use crate::chatlist_events; use crate::config::Config; use crate::constants::{ @@ -1647,6 +1647,7 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> m.param AS param, m.from_id AS from_id, m.rfc724_mid AS rfc724_mid, + c.archived AS archived, c.blocked AS blocked FROM msgs m LEFT JOIN chats c ON c.id=m.chat_id WHERE m.id IN ({}) AND m.chat_id>9", @@ -1660,16 +1661,20 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> let param: Params = row.get::<_, String>("param")?.parse().unwrap_or_default(); let from_id: ContactId = row.get("from_id")?; let rfc724_mid: String = row.get("rfc724_mid")?; + let visibility: ChatVisibility = row.get("archived")?; let blocked: Option = row.get("blocked")?; let ephemeral_timer: EphemeralTimer = row.get("ephemeral_timer")?; Ok(( - id, - chat_id, - state, - param, - from_id, - rfc724_mid, - blocked.unwrap_or_default(), + ( + id, + chat_id, + state, + param, + from_id, + rfc724_mid, + visibility, + blocked.unwrap_or_default(), + ), ephemeral_timer, )) }, @@ -1677,25 +1682,28 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> ) .await?; - if msgs.iter().any( - |(_id, _chat_id, _state, _param, _from_id, _rfc724_mid, _blocked, ephemeral_timer)| { - *ephemeral_timer != EphemeralTimer::Disabled - }, - ) { + if msgs + .iter() + .any(|(_, ephemeral_timer)| *ephemeral_timer != EphemeralTimer::Disabled) + { start_ephemeral_timers_msgids(context, &msg_ids) .await .context("failed to start ephemeral timers")?; } let mut updated_chat_ids = BTreeSet::new(); + let mut archived_chats_maybe_noticed = false; for ( - id, - curr_chat_id, - curr_state, - curr_param, - curr_from_id, - curr_rfc724_mid, - curr_blocked, + ( + id, + curr_chat_id, + curr_state, + curr_param, + curr_from_id, + curr_rfc724_mid, + curr_visibility, + curr_blocked, + ), _curr_ephemeral_timer, ) in msgs { @@ -1733,12 +1741,17 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> } updated_chat_ids.insert(curr_chat_id); } + archived_chats_maybe_noticed |= + curr_state == MessageState::InFresh && curr_visibility == ChatVisibility::Archived; } for updated_chat_id in updated_chat_ids { context.emit_event(EventType::MsgsNoticed(updated_chat_id)); chatlist_events::emit_chatlist_item_changed(context, updated_chat_id); } + if archived_chats_maybe_noticed { + context.on_archived_chats_maybe_noticed(); + } Ok(()) }