Update group test

python/tests/test_account.py

@@ -1177,29 +1177,46 @@ class TestOnlineAccount:
 class TestGroupStressTests:
     def test_group_many_members_add_leave_remove(self, acfactory, lp):
         lp.sec("creating and configuring five accounts")
-        ac1 = acfactory.get_online_configuring_account()
-        accounts = [acfactory.get_online_configuring_account() for i in range(3)]
-        wait_configuration_progress(ac1, 1000)
+        accounts = [acfactory.get_online_configuring_account() for i in range(5)]
         for acc in accounts:
             wait_configuration_progress(acc, 1000)
+        ac1 = accounts.pop()
+
+        lp.sec("ac1: setting up contacts with 4 other members")
+        contacts = []
+        for acc, name in zip(accounts, list("äöüsr")):
+            contact = ac1.create_contact(acc.get_config("addr"), name=name)
+            contacts.append(contact)
+
+            # make sure we accept the "hi" message
+            ac1.create_chat_by_contact(contact)
+
+            # make sure the other side accepts our messages
+            c1 = acc.create_contact(ac1.get_config("addr"), "ä member")
+            chat1 = acc.create_chat_by_contact(c1)
+
+            # send a message to get the contact key via autocrypt header
+            chat1.send_text("hi")
+            msg = ac1.wait_next_incoming_message()
+            assert msg.text == "hi"
+
+        # Save fifth account for later
+        ac5 = accounts.pop()
+        contact5 = contacts.pop()
 
         lp.sec("ac1: creating group chat with 3 other members")
         chat = ac1.create_group_chat("title1")
-        contacts = []
-        chars = list("äöüsr")
-        for acc in accounts:
-            contact = ac1.create_contact(acc.get_config("addr"), name=chars.pop())
-            contacts.append(contact)
+        for contact in contacts:
             chat.add_contact(contact)
-            # make sure the other side accepts our messages
-            c1 = acc.create_contact(ac1.get_config("addr"), "ä member")
-            acc.create_chat_by_contact(c1)
-
         assert not chat.is_promoted()
 
         lp.sec("ac1: send mesage to new group chat")
-        chat.send_text("hello")
+        msg = chat.send_text("hello")
         assert chat.is_promoted()
+        assert msg.is_encrypted()
+
+        gossiped_timestamp = chat.get_summary()["gossiped_timestamp"]
+        assert gossiped_timestamp > 0
 
         num_contacts = len(chat.get_contacts())
         assert num_contacts == 3 + 1
@@ -1215,19 +1232,45 @@ class TestGroupStressTests:
         ac3 = accounts[1]
         msg3 = ac3.wait_next_incoming_message()
         assert msg3.text == "hello"
-        contacts = ac3.get_contacts()
-        assert len(contacts) == 3
+        ac3_contacts = ac3.get_contacts()
+        assert len(ac3_contacts) == 3
         ac4_contacts = ac3.get_contacts(query=accounts[2].get_config("addr"))
         assert len(ac4_contacts) == 1
 
-        lp.sec("ac1: removing one contacts and checking things are right")
-        to_remove = msg.chat.get_contacts()[-1]
+        lp.sec("ac2: removing one contact")
+        to_remove = contacts[-1]
         msg.chat.remove_contact(to_remove)
 
+        lp.sec("ac1: receiving system message about contact removal")
         sysmsg = ac1.wait_next_incoming_message()
         assert to_remove.addr in sysmsg.text
         assert len(sysmsg.chat.get_contacts()) == 3
 
+        # Receiving message about removed contact does not reset gossip
+        assert chat.get_summary()["gossiped_timestamp"] == gossiped_timestamp
+
+        lp.sec("ac1: sending another message to the chat")
+        chat.send_text("hello2")
+        msg = ac2.wait_next_incoming_message()
+        assert msg.text == "hello2"
+        assert chat.get_summary()["gossiped_timestamp"] == gossiped_timestamp
+
+        lp.sec("ac1: adding fifth member to the chat")
+        chat.add_contact(contact5)
+        # Additng contact to chat resets gossiped_timestamp
+        assert chat.get_summary()["gossiped_timestamp"] >= gossiped_timestamp
+
+        lp.sec("ac2: receiving system message about contact addition")
+        sysmsg = ac2.wait_next_incoming_message()
+        assert contact5.addr in sysmsg.text
+        assert len(sysmsg.chat.get_contacts()) == 4
+
+        lp.sec("ac5: waiting for message about addition to the chat")
+        sysmsg = ac5.wait_next_incoming_message()
+        msg = sysmsg.chat.send_text("hello!")
+        # Message should be encrypted because keys of other members are gossiped
+        assert msg.is_encrypted()
+
 
 class TestOnlineConfigureFails:
     def test_invalid_password(self, acfactory):

src/chat.rs

@@ -1678,7 +1678,7 @@ fn real_group_exists(context: &Context, chat_id: u32) -> bool {
         .unwrap_or_default()
 }
 
-pub fn reset_gossiped_timestamp(context: &Context, chat_id: u32) -> crate::sql::Result<()> {
+pub fn reset_gossiped_timestamp(context: &Context, chat_id: u32) -> Result<(), Error> {
     set_gossiped_timestamp(context, chat_id, 0)
 }
 
@@ -1699,31 +1699,23 @@ pub fn set_gossiped_timestamp(
     context: &Context,
     chat_id: u32,
     timestamp: i64,
-) -> crate::sql::Result<()> {
-    if 0 != chat_id {
-        info!(
-            context,
-            "set gossiped_timestamp for chat #{} to {}.", chat_id, timestamp,
-        );
+) -> Result<(), Error> {
+    ensure!(
+        chat_id > DC_CHAT_ID_LAST_SPECIAL,
+        "can not add member to special chats"
+    );
+    info!(
+        context,
+        "set gossiped_timestamp for chat #{} to {}.", chat_id, timestamp,
+    );
 
-        sql::execute(
-            context,
-            &context.sql,
-            "UPDATE chats SET gossiped_timestamp=? WHERE id=?;",
-            params![timestamp, chat_id as i32],
-        )
-    } else {
-        info!(
-            context,
-            "set gossiped_timestamp for all chats to {}.", timestamp,
-        );
-        sql::execute(
-            context,
-            &context.sql,
-            "UPDATE chats SET gossiped_timestamp=?;",
-            params![timestamp],
-        )
-    }
+    sql::execute(
+        context,
+        &context.sql,
+        "UPDATE chats SET gossiped_timestamp=? WHERE id=?;",
+        params![timestamp, chat_id as i32],
+    )?;
+    Ok(())
 }
 
 pub fn shall_attach_selfavatar(context: &Context, chat_id: u32) -> Result<bool, Error> {

src/dc_receive_imf.rs

@@ -979,9 +979,14 @@ fn create_or_lookup_group(
     // XXX insert code in a different PR :)
 
     // execute group commands
-    if X_MrAddToGrp.is_some() || X_MrRemoveFromGrp.is_some() {
+    if X_MrAddToGrp.is_some() {
+        chat::reset_gossiped_timestamp(context, chat_id)?;
         recreate_member_list = true;
-    } else if X_MrGrpNameChanged {
+    }
+    if X_MrRemoveFromGrp.is_some() {
+        recreate_member_list = true;
+    }
+    if X_MrGrpNameChanged {
         if let Some(ref grpname) = grpname {
             if grpname.len() < 200 {
                 info!(context, "updating grpname for chat {}", chat_id);
@@ -1049,7 +1054,6 @@ fn create_or_lookup_group(
             }
         }
         send_EVENT_CHAT_MODIFIED = true;
-        chat::reset_gossiped_timestamp(context, chat_id)?;
     }
 
     if send_EVENT_CHAT_MODIFIED {

src/mimefactory.rs

@@ -305,7 +305,7 @@ impl<'a, 'b> MimeFactory<'a, 'b> {
                 let chat = self.chat.as_ref().unwrap();
                 // beside key- and member-changes, force re-gossip every 48 hours
                 let gossiped_timestamp = chat.get_gossiped_timestamp(self.context);
-                if gossiped_timestamp == 0 || (gossiped_timestamp + (2 * 24 * 60 * 60)) > time() {
+                if time() > gossiped_timestamp + (2 * 24 * 60 * 60) {
                     return true;
                 }
 

src/peerstate.rs

@@ -5,7 +5,6 @@ use std::fmt;
 use num_traits::FromPrimitive;
 
 use crate::aheader::*;
-use crate::chat::*;
 use crate::constants::*;
 use crate::context::Context;
 use crate::key::*;
@@ -418,7 +417,6 @@ impl<'a> Peerstate<'a> {
                     &self.addr,
                 ],
             )?;
-            reset_gossiped_timestamp(self.context, 0)?;
         } else if self.to_save == Some(ToSave::Timestamps) {
             sql::execute(
                 self.context,