From 95ca5ecd38c499593538af18b4d630f1a1d558e2 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Tue, 20 Jan 2026 18:31:23 -0300 Subject: [PATCH] feat: Remove imap::Session::sync_seen_flags() (#7742) --- deltachat-rpc-client/tests/test_something.py | 29 +++ src/imap.rs | 191 +------------------ src/imap/capabilities.rs | 4 - src/imap/client.rs | 1 - src/imap/imap_tests.rs | 1 + src/imap/select_folder.rs | 6 +- src/imap/session.rs | 4 - src/scheduler.rs | 8 - standards.md | 1 - 9 files changed, 34 insertions(+), 211 deletions(-) diff --git a/deltachat-rpc-client/tests/test_something.py b/deltachat-rpc-client/tests/test_something.py index fe329b4bf..200b31e2f 100644 --- a/deltachat-rpc-client/tests/test_something.py +++ b/deltachat-rpc-client/tests/test_something.py @@ -442,6 +442,35 @@ def test_reaction_seen_on_another_dev(acfactory) -> None: assert chat_id == alice2_chat_bob.id +def test_2nd_device_events_when_msgs_are_seen(acfactory) -> None: + alice, bob = acfactory.get_online_accounts(2) + alice2 = alice.clone() + alice2.start_io() + + # Get an accepted chat, otherwise alice2 won't be notified about the 2nd message. + chat_alice2 = alice2.create_chat(bob) + chat_id_alice2 = chat_alice2.get_basic_snapshot().id + + chat_bob_alice = bob.create_chat(alice) + chat_bob_alice.send_text("Hello!") + msg_alice = alice.wait_for_incoming_msg() + assert alice2.wait_for_incoming_msg_event().chat_id == chat_id_alice2 + chat_bob_alice.send_text("What's new?") + assert alice2.wait_for_incoming_msg_event().chat_id == chat_id_alice2 + chat_alice2 = alice2.get_chat_by_id(chat_id_alice2) + assert chat_alice2.get_fresh_message_count() == 2 + + msg_alice.mark_seen() + assert alice2.wait_for_msgs_changed_event().chat_id == chat_id_alice2 + assert chat_alice2.get_fresh_message_count() == 1 + + msg_id = alice.wait_for_msgs_changed_event().msg_id + msg = alice.get_message_by_id(msg_id) + msg.mark_seen() + assert alice2.wait_for_event(EventType.MSGS_NOTICED).chat_id == chat_id_alice2 + assert chat_alice2.get_fresh_message_count() == 0 + + def test_is_bot(acfactory) -> None: """Test that we can recognize messages submitted by bots.""" alice, bob = acfactory.get_online_accounts(2) diff --git a/src/imap.rs b/src/imap.rs index e03488527..fb2375083 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -6,7 +6,7 @@ use std::{ cmp::max, cmp::min, - collections::{BTreeMap, BTreeSet, HashMap}, + collections::{BTreeMap, HashMap}, iter::Peekable, mem::take, sync::atomic::Ordering, @@ -24,8 +24,7 @@ use url::Url; use crate::calls::{ UnresolvedIceServer, create_fallback_ice_servers, create_ice_servers_from_metadata, }; -use crate::chat::{self, ChatId, ChatIdBlocked, add_device_msg}; -use crate::chatlist_events; +use crate::chat::{self, ChatIdBlocked, add_device_msg}; use crate::config::Config; use crate::constants::{Blocked, DC_VERSION_STR}; use crate::contact::ContactId; @@ -34,7 +33,7 @@ use crate::ensure_and_debug_assert; use crate::events::EventType; use crate::headerdef::{HeaderDef, HeaderDefMap}; use crate::log::{LogExt, warn}; -use crate::message::{self, Message, MessageState, MsgId}; +use crate::message::{self, Message}; use crate::mimeparser; use crate::net::proxy::ProxyConfig; use crate::net::session::SessionStream; @@ -1173,114 +1172,6 @@ impl Session { Ok(()) } - /// Synchronizes `\Seen` flags using `CONDSTORE` extension. - pub(crate) async fn sync_seen_flags(&mut self, context: &Context, folder: &str) -> Result<()> { - if !self.can_condstore() { - info!( - context, - "Server does not support CONDSTORE, skipping flag synchronization." - ); - return Ok(()); - } - - if context.get_config_bool(Config::TeamProfile).await? { - return Ok(()); - } - - let folder_exists = self - .select_with_uidvalidity(context, folder) - .await - .context("Failed to select folder")?; - if !folder_exists { - return Ok(()); - } - - let mailbox = self - .selected_mailbox - .as_ref() - .with_context(|| format!("No mailbox selected, folder: {folder}"))?; - - // Check if the mailbox supports MODSEQ. - // We are not interested in actual value of HIGHESTMODSEQ. - if mailbox.highest_modseq.is_none() { - info!( - context, - "Mailbox {} does not support mod-sequences, skipping flag synchronization.", folder - ); - return Ok(()); - } - - let transport_id = self.transport_id(); - let mut updated_chat_ids = BTreeSet::new(); - let uid_validity = get_uidvalidity(context, transport_id, folder) - .await - .with_context(|| format!("failed to get UID validity for folder {folder}"))?; - let mut highest_modseq = get_modseq(context, transport_id, folder) - .await - .with_context(|| format!("failed to get MODSEQ for folder {folder}"))?; - let mut list = self - .uid_fetch("1:*", format!("(FLAGS) (CHANGEDSINCE {highest_modseq})")) - .await - .context("failed to fetch flags")?; - - let mut got_unsolicited_fetch = false; - - while let Some(fetch) = list - .try_next() - .await - .context("failed to get FETCH result")? - { - let uid = if let Some(uid) = fetch.uid { - uid - } else { - info!(context, "FETCH result contains no UID, skipping"); - got_unsolicited_fetch = true; - continue; - }; - let is_seen = fetch.flags().any(|flag| flag == Flag::Seen); - if is_seen - && let Some(chat_id) = mark_seen_by_uid(context, transport_id, folder, uid_validity, uid) - .await - .with_context(|| { - format!("Transport {transport_id}: Failed to update seen status for msg {folder}/{uid}") - })? - { - updated_chat_ids.insert(chat_id); - } - - if let Some(modseq) = fetch.modseq { - if modseq > highest_modseq { - highest_modseq = modseq; - } - } else { - warn!(context, "FETCH result contains no MODSEQ"); - } - } - drop(list); - - if got_unsolicited_fetch { - // We got unsolicited FETCH, which means some flags - // have been modified while our request was in progress. - // We may or may not have these new flags as a part of the response, - // so better skip next IDLE and do another round of flag synchronization. - info!(context, "Got unsolicited fetch, will skip idle"); - self.new_mail = true; - } - - set_modseq(context, transport_id, 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); - } - - Ok(()) - } - /// Fetches a list of messages by server UID. /// /// Sends pairs of UID and info about each downloaded message to the provided channel. @@ -2028,71 +1919,6 @@ pub(crate) async fn prefetch_should_download( Ok(should_download) } -/// Marks messages in `msgs` table as seen, searching for them by UID. -/// -/// Returns updated chat ID if any message was marked as seen. -async fn mark_seen_by_uid( - context: &Context, - transport_id: u32, - folder: &str, - uid_validity: u32, - uid: u32, -) -> Result> { - if let Some((msg_id, chat_id)) = context - .sql - .query_row_optional( - "SELECT id, chat_id FROM msgs - WHERE id > 9 AND rfc724_mid IN ( - SELECT rfc724_mid FROM imap - WHERE transport_id=? - AND folder=? - AND uidvalidity=? - AND uid=? - LIMIT 1 - )", - (transport_id, &folder, uid_validity, uid), - |row| { - let msg_id: MsgId = row.get(0)?; - let chat_id: ChatId = row.get(1)?; - Ok((msg_id, chat_id)) - }, - ) - .await - .with_context(|| format!("failed to get msg and chat ID for IMAP message {folder}/{uid}"))? - { - let updated = context - .sql - .execute( - "UPDATE msgs SET state=?1 - WHERE (state=?2 OR state=?3) - AND id=?4", - ( - MessageState::InSeen, - MessageState::InFresh, - MessageState::InNoticed, - msg_id, - ), - ) - .await - .with_context(|| format!("failed to update msg {msg_id} state"))? - > 0; - - if updated { - msg_id - .start_ephemeral_timer(context) - .await - .with_context(|| format!("failed to start ephemeral timer for message {msg_id}"))?; - Ok(Some(chat_id)) - } else { - // Message state has not changed. - Ok(None) - } - } else { - // There is no message is `msgs` table matching the given UID. - Ok(None) - } -} - /// Schedule marking the message as Seen on IMAP by adding all known IMAP messages corresponding to /// the given Message-ID to `imap_markseen` table. pub(crate) async fn markseen_on_imap_table(context: &Context, message_id: &str) -> Result<()> { @@ -2190,17 +2016,6 @@ pub(crate) async fn set_modseq( Ok(()) } -async fn get_modseq(context: &Context, transport_id: u32, folder: &str) -> Result { - Ok(context - .sql - .query_get_value( - "SELECT modseq FROM imap_sync WHERE transport_id=? AND folder=?", - (transport_id, folder), - ) - .await? - .unwrap_or(0)) -} - /// Builds a list of sequence/uid sets. The returned sets have each no more than around 1000 /// characters because according to /// command lines should not be much more than 1000 chars (servers should allow at least 8000 chars) diff --git a/src/imap/capabilities.rs b/src/imap/capabilities.rs index 3a216be31..d635fe518 100644 --- a/src/imap/capabilities.rs +++ b/src/imap/capabilities.rs @@ -17,10 +17,6 @@ pub(crate) struct Capabilities { /// pub can_check_quota: bool, - /// True if the server has CONDSTORE capability as defined in - /// - pub can_condstore: bool, - /// True if the server has METADATA capability as defined in /// pub can_metadata: bool, diff --git a/src/imap/client.rs b/src/imap/client.rs index adf6e8db1..fff839843 100644 --- a/src/imap/client.rs +++ b/src/imap/client.rs @@ -66,7 +66,6 @@ pub(crate) async fn determine_capabilities( can_idle: caps.has_str("IDLE"), can_move: caps.has_str("MOVE"), can_check_quota: caps.has_str("QUOTA"), - can_condstore: caps.has_str("CONDSTORE"), can_metadata: caps.has_str("METADATA"), can_compress: caps.has_str("COMPRESS=DEFLATE"), can_push: caps.has_str("XDELTAPUSH"), diff --git a/src/imap/imap_tests.rs b/src/imap/imap_tests.rs index 4bd9c939b..b492b0f8b 100644 --- a/src/imap/imap_tests.rs +++ b/src/imap/imap_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::chat::ChatId; use crate::contact::Contact; use crate::test_utils::TestContext; diff --git a/src/imap/select_folder.rs b/src/imap/select_folder.rs index 49fe1f878..78c743b33 100644 --- a/src/imap/select_folder.rs +++ b/src/imap/select_folder.rs @@ -65,11 +65,7 @@ impl ImapSession { self.maybe_close_folder(context).await?; // select new folder - let res = if self.can_condstore() { - self.select_condstore(folder).await - } else { - self.select(folder).await - }; + let res = self.select(folder).await; let transport_id = self.transport_id(); diff --git a/src/imap/session.rs b/src/imap/session.rs index 8d4e7e087..ea6102957 100644 --- a/src/imap/session.rs +++ b/src/imap/session.rs @@ -100,10 +100,6 @@ impl Session { self.capabilities.can_check_quota } - pub fn can_condstore(&self) -> bool { - self.capabilities.can_condstore - } - pub fn can_metadata(&self) -> bool { self.capabilities.can_metadata } diff --git a/src/scheduler.rs b/src/scheduler.rs index a19338b84..89dd9d562 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -533,14 +533,6 @@ async fn fetch_idle(ctx: &Context, connection: &mut Imap, mut session: Session) .await .context("download_msgs")?; - // Synchronize Seen flags. - session - .sync_seen_flags(ctx, &watch_folder) - .await - .context("sync_seen_flags") - .log_err(ctx) - .ok(); - connection.connectivity.set_idle(ctx); ctx.emit_event(EventType::ImapInboxIdle); diff --git a/standards.md b/standards.md index 48fc330a1..6c3cd79c3 100644 --- a/standards.md +++ b/standards.md @@ -13,7 +13,6 @@ Filename encoding | Encoded Words ([RFC 2047][]), Encoded Word Ex Identify server folders | IMAP LIST Extension ([RFC 6154][]) Push | IMAP IDLE ([RFC 2177][]) Quota | IMAP QUOTA extension ([RFC 2087][]) -Seen status synchronization | IMAP CONDSTORE extension ([RFC 7162][]) Client/server identification | IMAP ID extension ([RFC 2971][]) Authorization | OAuth2 ([RFC 6749][]) End-to-end encryption | [Autocrypt Level 1][], OpenPGP ([RFC 4880][]), Security Multiparts for MIME ([RFC 1847][]) and [“Mixed Up” Encryption repairing](https://datatracker.ietf.org/doc/html/draft-dkg-openpgp-pgpmime-message-mangling-00)