feat: Sync chat Blocked state across devices (#4817)

src/chat.rs

@@ -38,6 +38,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::tools::{
     buf_compress, create_id, create_outgoing_rfc724_mid, create_smeared_timestamp,
     create_smeared_timestamps, get_abs_path, gm2local_offset, improve_single_line_input,
@@ -250,7 +251,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).await?;
+                    chat.id.unblock(&context.nosync()).await?;
                 }
                 chat.id
             }
@@ -356,6 +357,7 @@ 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();
         let chat = Chat::load_from_db(context, self).await?;
 
         match chat.typ {
@@ -384,12 +386,22 @@ impl ChatId {
             }
         }
 
+        if !nosync {
+            chat.add_sync_item(context, ChatAction::Block).await?;
+        }
         Ok(())
     }
 
     /// Unblocks the chat.
     pub async fn unblock(self, context: &Context) -> Result<()> {
+        let (context, nosync) = &context.unwrap_nosync();
+
         self.set_blocked(context, Blocked::Not).await?;
+
+        if !nosync {
+            let chat = Chat::load_from_db(context, self).await?;
+            chat.add_sync_item(context, ChatAction::Unblock).await?;
+        }
         Ok(())
     }
 
@@ -397,6 +409,7 @@ 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();
         let chat = Chat::load_from_db(context, self).await?;
 
         match chat.typ {
@@ -431,6 +444,9 @@ impl ChatId {
             context.emit_event(EventType::ChatModified(self));
         }
 
+        if !nosync {
+            chat.add_sync_item(context, ChatAction::Accept).await?;
+        }
         Ok(())
     }
 
@@ -1269,7 +1285,8 @@ pub struct Chat {
     /// Whether the chat is archived or pinned.
     pub visibility: ChatVisibility,
 
-    /// Group ID.
+    /// Group ID. For [`Chattype::Mailinglist`] -- mailing list address. Empty for 1:1 chats and
+    /// ad-hoc groups.
     pub grpid: String,
 
     /// Whether the chat is blocked, unblocked or a contact request.
@@ -1826,6 +1843,42 @@ impl Chat {
         context.scheduler.interrupt_ephemeral_task().await;
         Ok(msg.id)
     }
+
+    /// Returns chat id for the purpose of synchronisation across devices.
+    async fn get_sync_id(&self, context: &Context) -> Result<Option<sync::ChatId>> {
+        match self.typ {
+            Chattype::Single => {
+                let mut r = None;
+                for contact_id in get_chat_contacts(context, self.id).await? {
+                    if contact_id == ContactId::SELF {
+                        continue;
+                    }
+                    if r.is_some() {
+                        return Ok(None);
+                    }
+                    let contact = Contact::get_by_id(context, contact_id).await?;
+                    r = Some(sync::ChatId::ContactAddr(contact.get_addr().to_string()));
+                }
+                Ok(r)
+            }
+            Chattype::Broadcast | Chattype::Group | Chattype::Mailinglist => {
+                if self.grpid.is_empty() {
+                    return Ok(None);
+                }
+                Ok(Some(sync::ChatId::Grpid(self.grpid.clone())))
+            }
+        }
+    }
+
+    /// Adds a chat action to the list of items to synchronise to other devices.
+    pub(crate) async fn add_sync_item(&self, context: &Context, action: ChatAction) -> Result<()> {
+        if let Some(id) = self.get_sync_id(context).await? {
+            context
+                .add_sync_item(SyncData::AlterChat(sync::AlterChatData { id, action }))
+                .await?;
+        }
+        Ok(())
+    }
 }
 
 /// Whether the chat is pinned or archived.
@@ -3962,6 +4015,41 @@ pub(crate) async fn update_msg_text_and_timestamp(
     Ok(())
 }
 
+impl Context {
+    /// Executes [`SyncData::AlterChat`] item sent by other device.
+    pub(crate) async fn sync_alter_chat(&self, data: &sync::AlterChatData) -> Result<()> {
+        let chat_id = match &data.id {
+            sync::ChatId::ContactAddr(addr) => {
+                let Some(contact_id) =
+                    Contact::lookup_id_by_addr_ex(self, addr, Origin::Unknown, None).await?
+                else {
+                    warn!(self, "sync_alter_chat: No contact for addr '{addr}'.");
+                    return Ok(());
+                };
+                let Some(chat_id) = ChatId::lookup_by_contact(self, contact_id).await? else {
+                    warn!(self, "sync_alter_chat: No chat for addr '{addr}'.");
+                    return Ok(());
+                };
+                chat_id
+            }
+            sync::ChatId::Grpid(grpid) => {
+                let Some((chat_id, ..)) = get_chat_id_by_grpid(self, grpid).await? else {
+                    warn!(self, "sync_alter_chat: No chat for grpid '{grpid}'.");
+                    return Ok(());
+                };
+                chat_id
+            }
+        };
+        match &data.action {
+            ChatAction::Block => chat_id.block(self).await,
+            ChatAction::Unblock => chat_id.unblock(self).await,
+            ChatAction::Accept => chat_id.accept(self).await,
+        }
+        .ok();
+        Ok(())
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/contact.rs

@@ -523,10 +523,24 @@ impl Contact {
     ///
     /// To validate an e-mail address independently of the contact database
     /// use `may_be_valid_addr()`.
+    ///
+    /// Returns the contact ID of the contact belonging to the e-mail address or 0 if there is no
+    /// contact that is or was introduced by an accepted contact.
     pub async fn lookup_id_by_addr(
         context: &Context,
         addr: &str,
         min_origin: Origin,
+    ) -> Result<Option<ContactId>> {
+        Self::lookup_id_by_addr_ex(context, addr, min_origin, Some(Blocked::Not)).await
+    }
+
+    /// The same as `lookup_id_by_addr()`, but internal function. Currently also allows looking up
+    /// not unblocked contacts.
+    pub(crate) async fn lookup_id_by_addr_ex(
+        context: &Context,
+        addr: &str,
+        min_origin: Origin,
+        blocked: Option<Blocked>,
     ) -> Result<Option<ContactId>> {
         if addr.is_empty() {
             bail!("lookup_id_by_addr: empty address");
@@ -543,8 +557,14 @@ impl Contact {
             .query_get_value(
                 "SELECT id FROM contacts \
             WHERE addr=?1 COLLATE NOCASE \
-            AND id>?2 AND origin>=?3 AND blocked=0;",
+            AND id>?2 AND origin>=?3 AND (? OR blocked=?)",
-                (&addr_normalized, ContactId::LAST_SPECIAL, min_origin as u32),
+                (
+                    &addr_normalized,
+                    ContactId::LAST_SPECIAL,
+                    min_origin as u32,
+                    blocked.is_none(),
+                    blocked.unwrap_or_default(),
+                ),
             )
             .await?;
         Ok(id)
@@ -1415,7 +1435,7 @@ WHERE type=? AND id IN (
             if let Some((chat_id, _, _)) =
                 chat::get_chat_id_by_grpid(context, &contact.addr).await?
             {
-                chat_id.unblock(context).await?;
+                chat_id.unblock(&context.nosync()).await?;
             }
         }
     }

src/context.rs

@@ -173,6 +173,7 @@ impl ContextBuilder {
 #[derive(Clone, Debug)]
 pub struct Context {
     pub(crate) inner: Arc<InnerContext>,
+    nosync: bool,
 }
 
 impl Deref for Context {
@@ -392,11 +393,34 @@ 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

@@ -887,7 +887,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).await?;
+                    chat_id.unblock(&context.nosync()).await?;
                     chat_id_blocked = Blocked::Not;
                 }
             }
@@ -919,7 +919,7 @@ async fn add_parts(
 
             if let Some(chat_id) = chat_id {
                 if Blocked::Not != chat_id_blocked {
-                    chat_id.unblock(context).await?;
+                    chat_id.unblock(&context.nosync()).await?;
                     // Not assigning `chat_id_blocked = Blocked::Not` to avoid unused_assignments warning.
                 }
             }

src/securejoin/bob.rs

@@ -179,7 +179,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).await?;
+                        chat_id.unblock(&context.nosync()).await?;
                         chat_id
                     }
                     None => {

src/sync.rs

@@ -5,7 +5,7 @@ use lettre_email::mime::{self};
 use lettre_email::PartBuilder;
 use serde::{Deserialize, Serialize};
 
-use crate::chat::{Chat, ChatId};
+use crate::chat::{self, Chat};
 use crate::config::Config;
 use crate::constants::Blocked;
 use crate::contact::ContactId;
@@ -13,10 +13,10 @@ use crate::context::Context;
 use crate::message::{Message, MsgId, Viewtype};
 use crate::mimeparser::SystemMessage;
 use crate::param::Param;
-use crate::sync::SyncData::{AddQrToken, DeleteQrToken};
+use crate::sync::SyncData::{AddQrToken, AlterChat, DeleteQrToken};
 use crate::token::Namespace;
 use crate::tools::time;
-use crate::{chat, stock_str, token};
+use crate::{stock_str, token};
 
 #[derive(Debug, Serialize, Deserialize)]
 pub(crate) struct QrTokenData {
@@ -25,10 +25,40 @@ pub(crate) struct QrTokenData {
     pub(crate) grpid: Option<String>,
 }
 
+#[derive(Debug, Serialize, Deserialize, PartialEq)]
+pub(crate) enum ChatId {
+    ContactAddr(String),
+    Grpid(String),
+    // NOTE: Ad-hoc groups lack an identifier that can be used across devices so
+    // block/mute/etc. actions on them are not synchronized to other devices.
+}
+
+#[derive(Debug, Serialize, Deserialize, PartialEq)]
+pub(crate) enum ChatAction {
+    Block,
+    // TODO: Actually unblocking a chat is not a public API. `Contact::unblock()` is what a user
+    // does actually, but it doesn't call `chat::ChatId::unblock()`. So, unblocking chats sync
+    // doesn't work now, but let it be implemented on chats nevertheless. The straightforward fix is
+    // to call `chat::ChatId::unblock()` in a context of user action.
+    //
+    // But it still works if a message is sent to a blocked contact because
+    // `chat::ChatId::unblock()` is called then.
+    Unblock,
+
+    Accept,
+}
+
+#[derive(Debug, Serialize, Deserialize)]
+pub(crate) struct AlterChatData {
+    pub(crate) id: ChatId,
+    pub(crate) action: ChatAction,
+}
+
 #[derive(Debug, Serialize, Deserialize)]
 pub(crate) enum SyncData {
     AddQrToken(QrTokenData),
     DeleteQrToken(QrTokenData),
+    AlterChat(AlterChatData),
 }
 
 #[derive(Debug, Serialize, Deserialize)]
@@ -67,7 +97,7 @@ impl Context {
     /// Adds most recent qr-code tokens for a given chat to the list of items to be synced.
     /// If device synchronization is disabled,
     /// no tokens exist or the chat is unpromoted, the function does nothing.
-    pub(crate) async fn sync_qr_code_tokens(&self, chat_id: Option<ChatId>) -> Result<()> {
+    pub(crate) async fn sync_qr_code_tokens(&self, chat_id: Option<chat::ChatId>) -> Result<()> {
         if !self.get_config_bool(Config::SyncMsgs).await? {
             return Ok(());
         }
@@ -118,7 +148,7 @@ impl Context {
     pub async fn send_sync_msg(&self) -> Result<Option<MsgId>> {
         if let Some((json, ids)) = self.build_sync_json().await? {
             let chat_id =
-                ChatId::create_for_contact_with_blocked(self, ContactId::SELF, Blocked::Yes)
+                chat::ChatId::create_for_contact_with_blocked(self, ContactId::SELF, Blocked::Yes)
                     .await?;
             let mut msg = Message {
                 chat_id,
@@ -215,7 +245,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.
-    pub(crate) async fn execute_sync_items(&self, items: &SyncItems) -> Result<()> {
+    async fn execute_sync_items_inner(&self, items: &SyncItems) -> Result<()> {
         info!(self, "executing {} sync item(s)", items.items.len());
         for item in &items.items {
             match &item.data {
@@ -243,10 +273,16 @@ impl Context {
                     token::delete(self, Namespace::InviteNumber, &token.invitenumber).await?;
                     token::delete(self, Namespace::Auth, &token.auth).await?;
                 }
+                AlterChat(data) => self.sync_alter_chat(data).await?,
             }
         }
         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)]
@@ -256,6 +292,7 @@ mod tests {
     use super::*;
     use crate::chat::Chat;
     use crate::chatlist::Chatlist;
+    use crate::contact::{Contact, Origin};
     use crate::test_utils::TestContext;
     use crate::token::Namespace;
 
@@ -277,6 +314,18 @@ mod tests {
 
         assert!(t.build_sync_json().await?.is_none());
 
+        // Having one test on `SyncData::AlterChat` is sufficient here as `AlterChatData` introduces
+        // enums inside items. Let's avoid in-depth testing of the serialiser here which is an
+        // external crate.
+        t.add_sync_item_with_timestamp(
+            SyncData::AlterChat(AlterChatData {
+                id: ChatId::ContactAddr("bob@example.net".to_string()),
+                action: ChatAction::Block,
+            }),
+            1631781315,
+        )
+        .await?;
+
         t.add_sync_item_with_timestamp(
             SyncData::AddQrToken(QrTokenData {
                 invitenumber: "testinvite".to_string(),
@@ -300,6 +349,7 @@ mod tests {
         assert_eq!(
             serialized,
             r#"{"items":[
+{"timestamp":1631781315,"data":{"AlterChat":{"id":{"ContactAddr":"bob@example.net"},"action":"Block"}}},
 {"timestamp":1631781316,"data":{"AddQrToken":{"invitenumber":"testinvite","auth":"testauth","grpid":"group123"}}},
 {"timestamp":1631781317,"data":{"DeleteQrToken":{"invitenumber":"123!?\":.;{}","auth":"456","grpid":null}}}
 ]}"#
@@ -310,7 +360,7 @@ mod tests {
         assert!(t.build_sync_json().await?.is_none());
 
         let sync_items = t.parse_sync_items(serialized)?;
-        assert_eq!(sync_items.items.len(), 2);
+        assert_eq!(sync_items.items.len(), 3);
 
         Ok(())
     }
@@ -368,6 +418,22 @@ mod tests {
             )
             .is_err()); // missing field
 
+        assert!(t.parse_sync_items(
+                r#"{"items":[{"timestamp":1631781318,"data":{"AlterChat":{"id":{"ContactAddr":"bob@example.net"},"action":"Burn"}}}]}"#.to_string(),
+            )
+            .is_err()); // Unknown enum value
+
+        // Test enums inside items
+        let sync_items = t.parse_sync_items(
+          r#"{"items":[{"timestamp":1631781318,"data":{"AlterChat":{"id":{"ContactAddr":"bob@example.net"},"action":"Block"}}}]}"#.to_string(),
+        )?;
+        assert_eq!(sync_items.items.len(), 1);
+        let AlterChat(AlterChatData { id, action }) = &sync_items.items.get(0).unwrap().data else {
+            bail!("bad item");
+        };
+        assert_eq!(*id, ChatId::ContactAddr("bob@example.net".to_string()));
+        assert_eq!(*action, ChatAction::Block);
+
         // empty item list is okay
         assert_eq!(
             t.parse_sync_items(r#"{"items":[]}"#.to_string())?
@@ -423,6 +489,7 @@ mod tests {
         let sync_items = t
             .parse_sync_items(
                 r#"{"items":[
+{"timestamp":1631781315,"data":{"AlterChat":{"id":{"ContactAddr":"bob@example.net"},"action":"Block"}}},
 {"timestamp":1631781316,"data":{"AddQrToken":{"invitenumber":"yip-in","auth":"a"}}},
 {"timestamp":1631781316,"data":{"DeleteQrToken":{"invitenumber":"in","auth":"delete unexistent, shall continue"}}},
 {"timestamp":1631781316,"data":{"AddQrToken":{"invitenumber":"in","auth":"yip-auth"}}},
@@ -435,6 +502,11 @@ mod tests {
             ?;
         t.execute_sync_items(&sync_items).await?;
 
+        assert!(
+            Contact::lookup_id_by_addr(&t, "bob@example.net", Origin::Unknown)
+                .await?
+                .is_none()
+        );
         assert!(token::exists(&t, Namespace::InviteNumber, "yip-in").await);
         assert!(token::exists(&t, Namespace::Auth, "yip-auth").await);
         assert!(!token::exists(&t, Namespace::Auth, "non-existent").await);
@@ -462,7 +534,7 @@ mod tests {
         // check that the used self-talk is not visible to the user
         // but that creation will still work (in this case, the chat is empty)
         assert_eq!(Chatlist::try_load(&alice, 0, None, None).await?.len(), 0);
-        let chat_id = ChatId::create_for_contact(&alice, ContactId::SELF).await?;
+        let chat_id = chat::ChatId::create_for_contact(&alice, ContactId::SELF).await?;
         let chat = Chat::load_from_db(&alice, chat_id).await?;
         assert!(chat.is_self_talk());
         assert_eq!(Chatlist::try_load(&alice, 0, None, None).await?.len(), 1);
@@ -485,4 +557,41 @@ mod tests {
 
         Ok(())
     }
+
+    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    async fn test_alter_chat() -> Result<()> {
+        let alices = [
+            TestContext::new_alice().await,
+            TestContext::new_alice().await,
+        ];
+        for a in &alices {
+            a.set_config_bool(Config::SyncMsgs, true).await?;
+        }
+        let bob = TestContext::new_bob().await;
+
+        let ba_chat = bob.create_chat(&alices[0]).await;
+        let sent_msg = bob.send_text(ba_chat.id, "hi").await;
+        let a0b_chat_id = alices[0].recv_msg(&sent_msg).await.chat_id;
+        alices[1].recv_msg(&sent_msg).await;
+
+        async fn sync(alices: &[TestContext]) -> Result<()> {
+            alices.get(0).unwrap().send_sync_msg().await?.unwrap();
+            let sent_msg = alices.get(0).unwrap().pop_sent_msg().await;
+            alices.get(1).unwrap().recv_msg(&sent_msg).await;
+            Ok(())
+        }
+
+        assert_eq!(alices[1].get_chat(&bob).await.blocked, Blocked::Request);
+        a0b_chat_id.accept(&alices[0]).await?;
+        sync(&alices).await?;
+        assert_eq!(alices[1].get_chat(&bob).await.blocked, Blocked::Not);
+        a0b_chat_id.block(&alices[0]).await?;
+        sync(&alices).await?;
+        assert_eq!(alices[1].get_chat(&bob).await.blocked, Blocked::Yes);
+        a0b_chat_id.unblock(&alices[0]).await?;
+        sync(&alices).await?;
+        assert_eq!(alices[1].get_chat(&bob).await.blocked, Blocked::Not);
+
+        Ok(())
+    }
 }