refactor: Replace Context::nosync flag with internal functions taking enum Sync (#4817)

src/chat.rs

@@ -21,7 +21,7 @@ use crate::constants::{
     Blocked, Chattype, DC_CHAT_ID_ALLDONE_HINT, DC_CHAT_ID_ARCHIVED_LINK, DC_CHAT_ID_LAST_SPECIAL,
     DC_CHAT_ID_TRASH, DC_RESEND_USER_AVATAR_DAYS,
 };
-use crate::contact::{Contact, ContactId, Origin, VerifiedStatus};
+use crate::contact::{self, Contact, ContactId, Origin, VerifiedStatus};
 use crate::context::Context;
 use crate::debug_logging::maybe_set_logging_xdc;
 use crate::download::DownloadState;
@@ -39,7 +39,7 @@ use crate::scheduler::InterruptInfo;
 use crate::smtp::send_msg_to_smtp;
 use crate::sql;
 use crate::stock_str;
-use crate::sync::{self, ChatAction, SyncData};
+use crate::sync::{self, ChatAction, Sync::*, SyncData};
 use crate::tools::{
     buf_compress, create_id, create_outgoing_rfc724_mid, create_smeared_timestamp,
     create_smeared_timestamps, get_abs_path, gm2local_offset, improve_single_line_input,
@@ -252,7 +252,7 @@ impl ChatId {
         let chat_id = match ChatIdBlocked::lookup_by_contact(context, contact_id).await? {
             Some(chat) => {
                 if create_blocked == Blocked::Not && chat.blocked != Blocked::Not {
-                    chat.id.unblock(&context.nosync()).await?;
+                    chat.id.unblock_ex(context, Nosync).await?;
                 }
                 chat.id
             }
@@ -358,7 +358,10 @@ impl ChatId {
 
     /// Blocks the chat as a result of explicit user action.
     pub async fn block(self, context: &Context) -> Result<()> {
-        let (context, nosync) = &context.unwrap_nosync();
+        self.block_ex(context, Sync).await
+    }
+
+    pub(crate) async fn block_ex(self, context: &Context, sync: sync::Sync) -> Result<()> {
         let chat = Chat::load_from_db(context, self).await?;
 
         match chat.typ {
@@ -372,7 +375,7 @@ impl ChatId {
                             context,
                             "Blocking the contact {contact_id} to block 1:1 chat."
                         );
-                        Contact::block(&context.nosync(), contact_id).await?;
+                        contact::set_blocked(context, Nosync, contact_id, true).await?;
                     }
                 }
             }
@@ -387,7 +390,7 @@ impl ChatId {
             }
         }
 
-        if !nosync {
+        if sync.into() {
             // NB: For a 1:1 chat this currently triggers `Contact::block()` on other devices.
             chat.add_sync_item(context, ChatAction::Block).await?;
         }
@@ -396,11 +399,13 @@ impl ChatId {
 
     /// Unblocks the chat.
     pub async fn unblock(self, context: &Context) -> Result<()> {
-        let (context, nosync) = &context.unwrap_nosync();
+        self.unblock_ex(context, Sync).await
+    }
 
+    pub(crate) async fn unblock_ex(self, context: &Context, sync: sync::Sync) -> Result<()> {
         self.set_blocked(context, Blocked::Not).await?;
 
-        if !nosync {
+        if sync.into() {
             let chat = Chat::load_from_db(context, self).await?;
             // TODO: For a 1:1 chat this currently triggers `Contact::unblock()` on other devices.
             // Maybe we should unblock the contact locally too, this would also resolve discrepancy
@@ -414,7 +419,10 @@ impl ChatId {
     ///
     /// Unblocks the chat and scales up origin of contacts.
     pub async fn accept(self, context: &Context) -> Result<()> {
-        let (context, nosync) = &context.unwrap_nosync();
+        self.accept_ex(context, Sync).await
+    }
+
+    pub(crate) async fn accept_ex(self, context: &Context, sync: sync::Sync) -> Result<()> {
         let chat = Chat::load_from_db(context, self).await?;
 
         match chat.typ {
@@ -449,7 +457,7 @@ impl ChatId {
             context.emit_event(EventType::ChatModified(self));
         }
 
-        if !nosync {
+        if sync.into() {
             chat.add_sync_item(context, ChatAction::Accept).await?;
         }
         Ok(())
@@ -554,12 +562,20 @@ impl ChatId {
 
     /// Archives or unarchives a chat.
     pub async fn set_visibility(self, context: &Context, visibility: ChatVisibility) -> Result<()> {
+        self.set_visibility_ex(context, Sync, visibility).await
+    }
+
+    pub(crate) async fn set_visibility_ex(
+        self,
+        context: &Context,
+        sync: sync::Sync,
+        visibility: ChatVisibility,
+    ) -> Result<()> {
         ensure!(
             !self.is_special(),
             "bad chat_id, can not be special chat: {}",
             self
         );
-        let (context, nosync) = &context.unwrap_nosync();
 
         context
             .sql
@@ -580,7 +596,7 @@ impl ChatId {
 
         context.emit_msgs_changed_without_ids();
 
-        if !nosync {
+        if sync.into() {
             let chat = Chat::load_from_db(context, self).await?;
             chat.add_sync_item(context, ChatAction::SetVisibility(visibility))
                 .await?;
@@ -3388,8 +3404,16 @@ impl rusqlite::types::FromSql for MuteDuration {
 
 /// Mutes the chat for a given duration or unmutes it.
 pub async fn set_muted(context: &Context, chat_id: ChatId, duration: MuteDuration) -> Result<()> {
+    set_muted_ex(context, Sync, chat_id, duration).await
+}
+
+pub(crate) async fn set_muted_ex(
+    context: &Context,
+    sync: sync::Sync,
+    chat_id: ChatId,
+    duration: MuteDuration,
+) -> Result<()> {
     ensure!(!chat_id.is_special(), "Invalid chat ID");
-    let (context, nosync) = &context.unwrap_nosync();
     context
         .sql
         .execute(
@@ -3399,7 +3423,7 @@ pub async fn set_muted(context: &Context, chat_id: ChatId, duration: MuteDuratio
         .await
         .context(format!("Failed to set mute duration for {chat_id}"))?;
     context.emit_event(EventType::ChatModified(chat_id));
-    if !nosync {
+    if sync.into() {
         let chat = Chat::load_from_db(context, chat_id).await?;
         chat.add_sync_item(context, ChatAction::SetMuted(duration))
             .await?;
@@ -4040,8 +4064,12 @@ impl Context {
                     return Ok(());
                 };
                 match &data.action {
-                    ChatAction::Block => return Contact::block(self, contact_id).await,
+                    ChatAction::Block => {
-                    ChatAction::Unblock => return Contact::unblock(self, contact_id).await,
+                        return contact::set_blocked(self, Nosync, contact_id, true).await
+                    }
+                    ChatAction::Unblock => {
+                        return contact::set_blocked(self, Nosync, contact_id, false).await
+                    }
                     _ => (),
                 }
                 let Some(chat_id) = ChatId::lookup_by_contact(self, contact_id).await? else {
@@ -4059,11 +4087,11 @@ impl Context {
             }
         };
         match &data.action {
-            ChatAction::Block => chat_id.block(self).await,
+            ChatAction::Block => chat_id.block_ex(self, Nosync).await,
-            ChatAction::Unblock => chat_id.unblock(self).await,
+            ChatAction::Unblock => chat_id.unblock_ex(self, Nosync).await,
-            ChatAction::Accept => chat_id.accept(self).await,
+            ChatAction::Accept => chat_id.accept_ex(self, Nosync).await,
-            ChatAction::SetVisibility(v) => chat_id.set_visibility(self, *v).await,
+            ChatAction::SetVisibility(v) => chat_id.set_visibility_ex(self, Nosync, *v).await,
-            ChatAction::SetMuted(duration) => set_muted(self, chat_id, *duration).await,
+            ChatAction::SetMuted(duration) => set_muted_ex(self, Nosync, chat_id, *duration).await,
         }
         .ok();
         Ok(())

src/contact.rs

@@ -32,7 +32,7 @@ use crate::mimeparser::AvatarAction;
 use crate::param::{Param, Params};
 use crate::peerstate::{Peerstate, PeerstateVerifiedStatus};
 use crate::sql::{self, params_iter};
-use crate::sync::{self, SyncData};
+use crate::sync::{self, Sync::*, SyncData};
 use crate::tools::{
     duration_to_str, get_abs_path, improve_single_line_input, strip_rtlo_characters, time,
     EmailAddress,
@@ -460,12 +460,12 @@ impl Contact {
 
     /// Block the given contact.
     pub async fn block(context: &Context, id: ContactId) -> Result<()> {
-        set_block_contact(context, id, true).await
+        set_blocked(context, Sync, id, true).await
     }
 
     /// Unblock the given contact.
     pub async fn unblock(context: &Context, id: ContactId) -> Result<()> {
-        set_block_contact(context, id, false).await
+        set_blocked(context, Sync, id, false).await
     }
 
     /// Add a single contact as a result of an _explicit_ user action.
@@ -495,7 +495,7 @@ impl Contact {
             }
         }
         if blocked {
-            Contact::unblock(&context.nosync(), contact_id).await?;
+            set_blocked(context, Nosync, contact_id, false).await?;
         }
 
         Ok(contact_id)
@@ -1384,8 +1384,9 @@ fn sanitize_name_and_addr(name: &str, addr: &str) -> (String, String) {
     }
 }
 
-async fn set_block_contact(
+pub(crate) async fn set_blocked(
     context: &Context,
+    sync: sync::Sync,
     contact_id: ContactId,
     new_blocking: bool,
 ) -> Result<()> {
@@ -1394,8 +1395,6 @@ async fn set_block_contact(
         "Can't block special contact {}",
         contact_id
     );
-    let (context, nosync) = &context.unwrap_nosync();
-
     let contact = Contact::get_by_id(context, contact_id).await?;
 
     if contact.blocked != new_blocking {
@@ -1437,11 +1436,11 @@ WHERE type=? AND id IN (
             if let Some((chat_id, _, _)) =
                 chat::get_chat_id_by_grpid(context, &contact.addr).await?
             {
-                chat_id.unblock(&context.nosync()).await?;
+                chat_id.unblock_ex(context, Nosync).await?;
             }
         }
 
-        if !nosync {
+        if sync.into() {
             let action = match new_blocking {
                 true => sync::ChatAction::Block,
                 false => sync::ChatAction::Unblock,

src/context.rs

@@ -173,7 +173,6 @@ impl ContextBuilder {
 #[derive(Clone, Debug)]
 pub struct Context {
     pub(crate) inner: Arc<InnerContext>,
-    nosync: bool,
 }
 
 impl Deref for Context {
@@ -393,34 +392,11 @@ impl Context {
 
         let ctx = Context {
             inner: Arc::new(inner),
-            nosync: false,
         };
 
         Ok(ctx)
     }
 
-    /// Returns a `Context` in which sending sync messages must be skipped. `Self::unwrap_nosync()`
-    /// should be used to check this.
-    pub(crate) fn nosync(&self) -> Self {
-        Self {
-            inner: self.inner.clone(),
-            nosync: true,
-        }
-    }
-
-    /// Checks if sending sync messages must be skipped. Returns the original context and the result
-    /// of the check. If it's `true`, calls to [`Self::add_sync_item()`] mustn't be done to prevent
-    /// extra/recursive synchronisation.
-    pub(crate) fn unwrap_nosync(&self) -> (Self, bool) {
-        (
-            Self {
-                inner: self.inner.clone(),
-                nosync: false,
-            },
-            self.nosync,
-        )
-    }
-
     /// Starts the IO scheduler.
     pub async fn start_io(&mut self) {
         if !self.is_configured().await.unwrap_or_default() {

src/receive_imf.rs

@@ -36,6 +36,7 @@ use crate::securejoin::{self, handle_securejoin_handshake, observe_securejoin_on
 use crate::simplify;
 use crate::sql;
 use crate::stock_str;
+use crate::sync::Sync::*;
 use crate::tools::{
     buf_compress, extract_grpid_from_rfc724_mid, smeared_time, strip_rtlo_characters,
 };
@@ -887,7 +888,7 @@ async fn add_parts(
             // automatically unblock chat when the user sends a message
             if chat_id_blocked != Blocked::Not {
                 if let Some(chat_id) = chat_id {
-                    chat_id.unblock(&context.nosync()).await?;
+                    chat_id.unblock_ex(context, Nosync).await?;
                     chat_id_blocked = Blocked::Not;
                 }
             }
@@ -919,7 +920,7 @@ async fn add_parts(
 
             if let Some(chat_id) = chat_id {
                 if Blocked::Not != chat_id_blocked {
-                    chat_id.unblock(&context.nosync()).await?;
+                    chat_id.unblock_ex(context, Nosync).await?;
                     // Not assigning `chat_id_blocked = Blocked::Not` to avoid unused_assignments warning.
                 }
             }

src/securejoin/bob.rs

@@ -15,6 +15,7 @@ use crate::contact::Contact;
 use crate::context::Context;
 use crate::events::EventType;
 use crate::mimeparser::MimeMessage;
+use crate::sync::Sync::*;
 use crate::tools::time;
 use crate::{chat, stock_str};
 
@@ -179,7 +180,7 @@ impl BobState {
             } => {
                 let group_chat_id = match chat::get_chat_id_by_grpid(context, grpid).await? {
                     Some((chat_id, _protected, _blocked)) => {
-                        chat_id.unblock(&context.nosync()).await?;
+                        chat_id.unblock_ex(context, Nosync).await?;
                         chat_id
                     }
                     None => {

src/sync.rs

@@ -18,6 +18,22 @@ use crate::token::Namespace;
 use crate::tools::time;
 use crate::{stock_str, token};
 
+/// Whether to send device sync messages. Aimed for usage in the internal API.
+#[derive(Debug)]
+pub(crate) enum Sync {
+    Nosync,
+    Sync,
+}
+
+impl From<Sync> for bool {
+    fn from(sync: Sync) -> bool {
+        match sync {
+            Sync::Nosync => false,
+            Sync::Sync => true,
+        }
+    }
+}
+
 #[derive(Debug, Serialize, Deserialize)]
 pub(crate) struct QrTokenData {
     pub(crate) invitenumber: String,
@@ -228,7 +244,7 @@ impl Context {
         Ok(sync_items)
     }
 
-    /// Execute sync items.
+    /// Executes sync items sent by other device.
     ///
     /// CAVE: When changing the code to handle other sync items,
     /// take care that does not result in calls to `add_sync_item()`
@@ -239,7 +255,7 @@ impl Context {
     /// Therefore, errors should only be returned on database errors or so.
     /// If eg. just an item cannot be deleted,
     /// that should not hold off the other items to be executed.
-    async fn execute_sync_items_inner(&self, items: &SyncItems) -> Result<()> {
+    pub(crate) async fn execute_sync_items(&self, items: &SyncItems) -> Result<()> {
         info!(self, "executing {} sync item(s)", items.items.len());
         for item in &items.items {
             match &item.data {
@@ -272,11 +288,6 @@ impl Context {
         }
         Ok(())
     }
-
-    /// Executes sync items sent by other device.
-    pub(crate) async fn execute_sync_items(&self, items: &SyncItems) -> Result<()> {
-        self.nosync().execute_sync_items_inner(items).await
-    }
 }
 
 #[cfg(test)]