From 86e329741492c490742a29cdf800b5b9902eef80 Mon Sep 17 00:00:00 2001
From: bjoern <r10s@b44t.com>
Date: Wed, 1 Dec 2021 21:06:56 +0100
Subject: [PATCH] execute `Chat-Group-Member-Removed` even when arrived out of
 oder (#2857)

* execute Chat-Group-Member-Removed even when arrived out of oder

* test adding/removing members unordered
---
 src/chat.rs           | 62 +++++++++++++++++++++++++++++++++++++++++++
 src/dc_receive_imf.rs | 15 ++++++-----
 2 files changed, 70 insertions(+), 7 deletions(-)

diff --git a/src/chat.rs b/src/chat.rs
index 0b71b382a..fe8a18c5c 100644
--- a/src/chat.rs
+++ b/src/chat.rs
@@ -3304,6 +3304,68 @@ mod tests {
         Ok(())
     }
 
+    #[async_std::test]
+    async fn test_modify_chat_disordered() -> Result<()> {
+        // Alice creates a group with Bob, Claire and Daisy and then removes Claire and Daisy
+        // (sleep() is needed as otherwise smeared time from Alice looks to Bob like messages from the future which are all set to "now" then)
+        let alice = TestContext::new_alice().await;
+
+        let bob_id = Contact::create(&alice, "", "bob@example.net").await?;
+        let claire_id = Contact::create(&alice, "", "claire@foo.de").await?;
+        let daisy_id = Contact::create(&alice, "", "daisy@bar.de").await?;
+
+        let alice_chat_id = create_group_chat(&alice, ProtectionStatus::Unprotected, "foo").await?;
+        send_text_msg(&alice, alice_chat_id, "populate".to_string()).await?;
+
+        add_contact_to_chat(&alice, alice_chat_id, bob_id).await?;
+        async_std::task::sleep(std::time::Duration::from_millis(1100)).await;
+        let add1 = alice.pop_sent_msg().await;
+
+        add_contact_to_chat(&alice, alice_chat_id, claire_id).await?;
+        let add2 = alice.pop_sent_msg().await;
+        async_std::task::sleep(std::time::Duration::from_millis(1100)).await;
+
+        add_contact_to_chat(&alice, alice_chat_id, daisy_id).await?;
+        let add3 = alice.pop_sent_msg().await;
+        async_std::task::sleep(std::time::Duration::from_millis(1100)).await;
+
+        assert_eq!(get_chat_contacts(&alice, alice_chat_id).await?.len(), 4);
+
+        remove_contact_from_chat(&alice, alice_chat_id, claire_id).await?;
+        let remove1 = alice.pop_sent_msg().await;
+        async_std::task::sleep(std::time::Duration::from_millis(1100)).await;
+
+        remove_contact_from_chat(&alice, alice_chat_id, daisy_id).await?;
+        let remove2 = alice.pop_sent_msg().await;
+        async_std::task::sleep(std::time::Duration::from_millis(1100)).await;
+
+        assert_eq!(get_chat_contacts(&alice, alice_chat_id).await?.len(), 2);
+
+        // Bob receives the add and deletion messages out of order
+        let bob = TestContext::new_bob().await;
+        bob.recv_msg(&add1).await;
+        async_std::task::sleep(std::time::Duration::from_millis(1100)).await;
+
+        bob.recv_msg(&add3).await;
+        async_std::task::sleep(std::time::Duration::from_millis(1100)).await;
+
+        bob.recv_msg(&add2).await;
+        async_std::task::sleep(std::time::Duration::from_millis(1100)).await;
+
+        let bob_chat_id = bob.get_last_msg().await.chat_id;
+        assert_eq!(get_chat_contacts(&bob, bob_chat_id).await?.len(), 4);
+
+        bob.recv_msg(&remove2).await;
+        async_std::task::sleep(std::time::Duration::from_millis(1100)).await;
+
+        bob.recv_msg(&remove1).await;
+        async_std::task::sleep(std::time::Duration::from_millis(1100)).await;
+
+        assert_eq!(get_chat_contacts(&bob, bob_chat_id).await?.len(), 2);
+
+        Ok(())
+    }
+
     #[async_std::test]
     async fn test_add_remove_contact_for_single() {
         let ctx = TestContext::new_alice().await;
diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs
index 1041cc845..59c0f8d98 100644
--- a/src/dc_receive_imf.rs
+++ b/src/dc_receive_imf.rs
@@ -1706,13 +1706,14 @@ async fn apply_group_changes(
             send_event_chat_modified = true;
         }
     } else if let Some(contact_id) = removed_id {
-        if chat_id
-            .update_timestamp(context, Param::MemberListTimestamp, sent_timestamp)
-            .await?
-        {
-            chat::remove_from_chat_contacts_table(context, chat_id, contact_id).await?;
-            send_event_chat_modified = true;
-        }
+        // in contrast to `Chat-Group-Member-Added` we do not reconstruct the whole state from To: on removing -
+        // otherwise out-of-order removals won't work as members are re-added quickly.
+        //
+        // therefore, we need to ignore `MemberListTimestamp`
+        // and execute `Chat-Group-Member-Removed` statements even when they arrive out of order.
+        // we do not set `MemberListTimestamp` as we do not reconstuct the memberlist.
+        chat::remove_from_chat_contacts_table(context, chat_id, contact_id).await?;
+        send_event_chat_modified = true;
     }
 
     if send_event_chat_modified {