sync chat contacts (#4953)

TODO: tests
TODO: sync creating chats

src/chat.rs

@@ -1,6 +1,6 @@
 //! # Chat module.
 
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::convert::{TryFrom, TryInto};
 use std::fmt;
 use std::path::{Path, PathBuf};
@@ -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::{self, Contact, ContactId, Origin, VerifiedStatus};
+use crate::contact::{self, Contact, ContactAddress, ContactId, Origin, VerifiedStatus};
 use crate::context::Context;
 use crate::debug_logging::maybe_set_logging_xdc;
 use crate::download::DownloadState;
@@ -1891,6 +1891,17 @@ impl Chat {
         Ok(msg.id)
     }
 
+    /// Sends a `ChatAction` synchronising chat contacts to other devices.
+    pub(crate) async fn sync_contacts(&self, context: &Context) -> Result<()> {
+        let mut addrs = Vec::new();
+        for contact_id in get_chat_contacts(context, self.id).await? {
+            let contact = Contact::get_by_id(context, contact_id).await?;
+            addrs.push(contact.get_addr().to_string());
+        }
+        self.add_sync_item(context, ChatAction::SetContacts(addrs))
+            .await
+    }
+
     /// 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 {
@@ -1917,7 +1928,7 @@ impl Chat {
         }
     }
 
-    /// Adds a chat action to the list of items to synchronise to other devices.
+    /// Sends a chat action for synchronisation 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
@@ -3213,6 +3224,28 @@ pub async fn create_broadcast_list(context: &Context) -> Result<ChatId> {
     Ok(chat_id)
 }
 
+/// Set chat contacts in the `chats_contacts` table.
+pub(crate) async fn update_chat_contacts_table(
+    context: &Context,
+    id: ChatId,
+    contacts: &HashSet<ContactId>,
+) -> Result<()> {
+    context
+        .sql
+        .transaction(move |transaction| {
+            transaction.execute("DELETE FROM chats_contacts WHERE chat_id=?", (id,))?;
+            for contact_id in contacts {
+                transaction.execute(
+                    "INSERT INTO chats_contacts (chat_id, contact_id) VALUES(?, ?)",
+                    (id, contact_id),
+                )?;
+            }
+            Ok(())
+        })
+        .await?;
+    Ok(())
+}
+
 /// Adds contacts to the `chats_contacts` table.
 pub(crate) async fn add_to_chat_contacts_table(
     context: &Context,
@@ -3258,12 +3291,13 @@ pub async fn add_contact_to_chat(
     chat_id: ChatId,
     contact_id: ContactId,
 ) -> Result<()> {
-    add_contact_to_chat_ex(context, chat_id, contact_id, false).await?;
+    add_contact_to_chat_ex(context, Sync, chat_id, contact_id, false).await?;
     Ok(())
 }
 
 pub(crate) async fn add_contact_to_chat_ex(
     context: &Context,
+    mut sync: sync::Sync,
     chat_id: ChatId,
     contact_id: ContactId,
     from_handshake: bool,
@@ -3345,8 +3379,12 @@ pub(crate) async fn add_contact_to_chat_ex(
         msg.param.set(Param::Arg, contact_addr);
         msg.param.set_int(Param::Arg2, from_handshake.into());
         msg.id = send_msg(context, chat_id, &mut msg).await?;
+        sync = Nosync;
     }
     context.emit_event(EventType::ChatModified(chat_id));
+    if sync.into() {
+        chat.sync_contacts(context).await?;
+    }
     Ok(true)
 }
 
@@ -3486,6 +3524,7 @@ pub async fn remove_contact_from_chat(
             context.emit_event(EventType::ErrorSelfNotInGroup(err_msg.clone()));
             bail!("{}", err_msg);
         } else {
+            let mut sync = Nosync;
             // We do not return an error if the contact does not exist in the database.
             // This allows to delete dangling references to deleted contacts
             // in case of the database becoming inconsistent due to a bug.
@@ -3506,6 +3545,8 @@ pub async fn remove_contact_from_chat(
                     msg.param.set_cmd(SystemMessage::MemberRemovedFromGroup);
                     msg.param.set(Param::Arg, contact.get_addr());
                     msg.id = send_msg(context, chat_id, &mut msg).await?;
+                } else {
+                    sync = Sync;
                 }
             }
             // we remove the member from the chat after constructing the
@@ -3520,6 +3561,9 @@ pub async fn remove_contact_from_chat(
             // check/encryption logic.
             remove_from_chat_contacts_table(context, chat_id, contact_id).await?;
             context.emit_event(EventType::ChatModified(chat_id));
+            if sync.into() {
+                chat.sync_contacts(context).await?;
+            }
         }
     } else {
         bail!("Cannot remove members from non-group chats.");
@@ -4080,6 +4124,35 @@ pub(crate) async fn update_msg_text_and_timestamp(
     Ok(())
 }
 
+/// Set chat contacts by their addresses creating the corresponding contacts if necessary.
+pub(crate) async fn set_contacts_by_addrs(
+    context: &Context,
+    id: ChatId,
+    addrs: &[String],
+) -> Result<()> {
+    let chat = Chat::load_from_db(context, id).await?;
+    ensure!(
+        chat.typ == Chattype::Group || chat.typ == Chattype::Broadcast,
+        "{} is not a group/broadcast",
+        id,
+    );
+    let contacts_old = HashSet::<ContactId>::from_iter(get_chat_contacts(context, id).await?);
+    let mut contacts = HashSet::new();
+    for addr in addrs {
+        let contact =
+            Contact::add_or_lookup(context, "", ContactAddress::new(addr)?, Origin::Unknown)
+                .await?
+                .0;
+        contacts.insert(contact);
+    }
+    if contacts == contacts_old {
+        return Ok(());
+    }
+    update_chat_contacts_table(context, id, &contacts).await?;
+    context.emit_event(EventType::ChatModified(id));
+    Ok(())
+}
+
 impl Context {
     /// Executes [`SyncData::AlterChat`] item sent by other device.
     pub(crate) async fn sync_alter_chat(
@@ -4124,6 +4197,7 @@ impl Context {
             ChatAction::Accept => chat_id.accept_ex(self, Nosync).await,
             ChatAction::SetVisibility(v) => chat_id.set_visibility_ex(self, Nosync, *v).await,
             ChatAction::SetMuted(duration) => set_muted_ex(self, Nosync, chat_id, *duration).await,
+            ChatAction::SetContacts(addrs) => set_contacts_by_addrs(self, chat_id, addrs).await,
         }
         .ok();
         Ok(())
@@ -4349,7 +4423,7 @@ mod tests {
         let chat_id = create_group_chat(&t, ProtectionStatus::Unprotected, "foo")
             .await
             .unwrap();
-        let added = add_contact_to_chat_ex(&t, chat_id, ContactId::SELF, false)
+        let added = add_contact_to_chat_ex(&t, Nosync, chat_id, ContactId::SELF, false)
             .await
             .unwrap();
         assert_eq!(added, false);
@@ -4699,7 +4773,7 @@ mod tests {
 
         // adding or removing contacts from one-to-one-chats result in an error
         let claire = Contact::create(&ctx, "", "claire@foo.de").await.unwrap();
-        let added = add_contact_to_chat_ex(&ctx, chat.id, claire, false).await;
+        let added = add_contact_to_chat_ex(&ctx, Nosync, chat.id, claire, false).await;
         assert!(added.is_err());
         assert_eq!(get_chat_contacts(&ctx, chat.id).await.unwrap().len(), 1);
 

src/receive_imf.rs

@@ -1917,21 +1917,7 @@ async fn apply_group_changes(
         }
 
         if new_members != chat_contacts {
-            let new_members_ref = &new_members;
-            context
-                .sql
-                .transaction(move |transaction| {
-                    transaction
-                        .execute("DELETE FROM chats_contacts WHERE chat_id=?", (chat_id,))?;
-                    for contact_id in new_members_ref {
-                        transaction.execute(
-                            "INSERT INTO chats_contacts (chat_id, contact_id) VALUES(?, ?)",
-                            (chat_id, contact_id),
-                        )?;
-                    }
-                    Ok(())
-                })
-                .await?;
+            chat::update_chat_contacts_table(context, chat_id, &new_members).await?;
             chat_contacts = new_members;
             send_event_chat_modified = true;
         }

src/securejoin.rs

@@ -21,6 +21,7 @@ use crate::param::Param;
 use crate::peerstate::{Peerstate, PeerstateKeyType};
 use crate::qr::check_qr;
 use crate::stock_str;
+use crate::sync::Sync::*;
 use crate::token;
 use crate::tools::time;
 
@@ -445,8 +446,14 @@ pub(crate) async fn handle_securejoin_handshake(
                 match chat::get_chat_id_by_grpid(context, field_grpid).await? {
                     Some((group_chat_id, _, _)) => {
                         secure_connection_established(context, contact_id, group_chat_id).await?;
-                        chat::add_contact_to_chat_ex(context, group_chat_id, contact_id, true)
-                            .await?;
+                        chat::add_contact_to_chat_ex(
+                            context,
+                            Nosync,
+                            group_chat_id,
+                            contact_id,
+                            true,
+                        )
+                        .await?;
                     }
                     None => bail!("Chat {} not found", &field_grpid),
                 }

src/sync.rs

@@ -56,6 +56,8 @@ pub(crate) enum ChatAction {
     Accept,
     SetVisibility(ChatVisibility),
     SetMuted(chat::MuteDuration),
+    /// Set chat contacts by their addresses.
+    SetContacts(Vec<String>),
 }
 
 #[derive(Debug, Serialize, Deserialize)]