fix: Add Chat-Group-Name-Timestamp header and use it to update group names (#6412)

Add "Chat-Group-Name-Timestamp" message header and use the last-write-wins logic when updating group
names (similar to group member timestamps). Note that if the "Chat-Group-Name-Changed" header is
absent though, we don't add a system message (`MsgGrpNameChangedBy`) because we don't want to blame
anyone.

Author: iequidoo

src/chat.rs

@@ -3039,6 +3039,11 @@ pub(crate) async fn create_send_msg_jobs(context: &Context, msg: &mut Message) -
         msg.state = MessageState::OutDelivered;
         return Ok(Vec::new());
     }
+    if msg.param.get_cmd() == SystemMessage::GroupNameChanged {
+        msg.chat_id
+            .update_timestamp(context, Param::GroupNameTimestamp, msg.timestamp_sort)
+            .await?;
+    }
 
     let rendered_msg = match mimefactory.render(context).await {
         Ok(res) => Ok(res),

src/headerdef.rs

@@ -57,6 +57,7 @@ pub enum HeaderDef {
     ChatGroupId,
     ChatGroupName,
     ChatGroupNameChanged,
+    ChatGroupNameTimestamp,
     ChatVerified,
     ChatGroupAvatar,
     ChatUserAvatar,

src/mimefactory.rs

@@ -1190,6 +1190,12 @@ impl MimeFactory {
                 "Chat-Group-Name",
                 mail_builder::headers::text::Text::new(chat.name.to_string()).into(),
             ));
+            if let Some(ts) = chat.param.get_i64(Param::GroupNameTimestamp) {
+                headers.push((
+                    "Chat-Group-Name-Timestamp",
+                    mail_builder::headers::text::Text::new(ts.to_string()).into(),
+                ));
+            }
 
             match command {
                 SystemMessage::MemberRemovedFromGroup => {

src/mimeparser.rs

@@ -446,6 +446,7 @@ impl MimeMessage {
                     HeaderDef::ChatGroupId,
                     HeaderDef::ChatGroupName,
                     HeaderDef::ChatGroupNameChanged,
+                    HeaderDef::ChatGroupNameTimestamp,
                     HeaderDef::ChatGroupAvatar,
                     HeaderDef::ChatGroupMemberRemoved,
                     HeaderDef::ChatGroupMemberAdded,

src/receive_imf.rs

@@ -2411,9 +2411,19 @@ async fn apply_group_changes(
         }
 
         better_msg = Some(stock_str::msg_add_member_local(context, added_addr, from_id).await);
-    } else if let Some(old_name) = mime_parser
+    }
+
+    let group_name_timestamp = mime_parser
+        .get_header(HeaderDef::ChatGroupNameTimestamp)
+        .and_then(|s| s.parse::<i64>().ok());
+    if let Some(old_name) = mime_parser
         .get_header(HeaderDef::ChatGroupNameChanged)
         .map(|s| s.trim())
+        .or(match group_name_timestamp {
+            Some(0) => None,
+            Some(_) => Some(chat.name.as_str()),
+            None => None,
+        })
     {
         if let Some(grpname) = mime_parser
             .get_header(HeaderDef::ChatGroupName)
@@ -2422,13 +2432,15 @@ async fn apply_group_changes(
         {
             let grpname = &sanitize_single_line(grpname);
             let old_name = &sanitize_single_line(old_name);
-            if chat_id
-                .update_timestamp(
-                    context,
-                    Param::GroupNameTimestamp,
-                    mime_parser.timestamp_sent,
-                )
-                .await?
+
+            let chat_group_name_timestamp =
+                chat.param.get_i64(Param::GroupNameTimestamp).unwrap_or(0);
+            let group_name_timestamp = group_name_timestamp.unwrap_or(mime_parser.timestamp_sent);
+            // To provide group name consistency, compare names if timestamps are equal.
+            if (chat_group_name_timestamp, grpname) < (group_name_timestamp, old_name)
+                && chat_id
+                    .update_timestamp(context, Param::GroupNameTimestamp, group_name_timestamp)
+                    .await?
             {
                 info!(context, "Updating grpname for chat {chat_id}.");
                 context
@@ -2437,10 +2449,18 @@ async fn apply_group_changes(
                     .await?;
                 send_event_chat_modified = true;
             }
-
-            better_msg = Some(stock_str::msg_grp_name(context, old_name, grpname, from_id).await);
+            if mime_parser
+                .get_header(HeaderDef::ChatGroupNameChanged)
+                .is_some()
+            {
+                better_msg.get_or_insert(
+                    stock_str::msg_grp_name(context, old_name, grpname, from_id).await,
+                );
+            }
         }
-    } else if let Some(value) = mime_parser.get_header(HeaderDef::ChatContent) {
+    }
+
+    if let (Some(value), None) = (mime_parser.get_header(HeaderDef::ChatContent), &better_msg) {
         if value == "group-avatar-changed" {
             if let Some(avatar_action) = &mime_parser.group_avatar {
                 // this is just an explicit message containing the group-avatar,

src/receive_imf/receive_imf_tests.rs

@@ -5428,6 +5428,40 @@ Hello!"
     Ok(())
 }
 
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_rename_chat_on_missing_message() -> Result<()> {
+    let alice = TestContext::new_alice().await;
+    let bob = TestContext::new_bob().await;
+    let chat_id = create_group_chat(&alice, ProtectionStatus::Unprotected, "Group").await?;
+    add_to_chat_contacts_table(
+        &alice,
+        time(),
+        chat_id,
+        &[Contact::create(&alice, "bob", "[email protected]").await?],
+    )
+    .await?;
+    send_text_msg(&alice, chat_id, "populate".to_string()).await?;
+    let bob_chat_id = bob.recv_msg(&alice.pop_sent_msg().await).await.chat_id;
+    bob_chat_id.accept(&bob).await?;
+
+    // Bob changes the group name. NB: If Bob does this too fast, it's not guaranteed that his group
+    // name wins because "Group-Name-Timestamp" may not increase.
+    SystemTime::shift(Duration::from_secs(3600));
+    chat::set_chat_name(&bob, bob_chat_id, "Renamed").await?;
+    bob.pop_sent_msg().await;
+
+    // Bob adds a new member.
+    let bob_orange = Contact::create(&bob, "orange", "[email protected]").await?;
+    add_contact_to_chat(&bob, bob_chat_id, bob_orange).await?;
+    let add_msg = bob.pop_sent_msg().await;
+
+    // Alice only receives the member addition.
+    alice.recv_msg(&add_msg).await;
+    let chat = Chat::load_from_db(&alice, chat_id).await?;
+    assert_eq!(chat.get_name(), "Renamed");
+    Ok(())
+}
+
 /// Tests that creating a group
 /// is preferred over assigning message to existing
 /// chat based on `In-Reply-To` and `References`.

src/update_helper.rs

@@ -213,6 +213,44 @@ mod tests {
         // Assert that the \n was correctly removed from the group name also in the system message
         assert_eq!(msg.text.contains('\n'), false);
 
+        // This doesn't update the name because Date is the same and name is greater.
+        receive_imf(
+            &t,
+            b"From: Bob Authname <[email protected]>\n\
+                 To: [email protected]\n\
+                 Message-ID: <[email protected]>\n\
+                 Chat-Version: 1.0\n\
+                 Chat-Group-ID: abcde123456\n\
+                 Chat-Group-Name: another name update 4\n\
+                 Chat-Group-Name-Changed: another name update\n\
+                 Date: Sun, 22 Mar 2021 03:00:00 +0000\n\
+                 \n\
+                 4th message\n",
+            false,
+        )
+        .await?;
+        let chat = Chat::load_from_db(&t, chat.id).await?;
+        assert_eq!(chat.name, "another name update");
+
+        // This updates the name because Date is the same and name is lower.
+        receive_imf(
+            &t,
+            b"From: Bob Authname <[email protected]>\n\
+                 To: [email protected]\n\
+                 Message-ID: <[email protected]>\n\
+                 Chat-Version: 1.0\n\
+                 Chat-Group-ID: abcde123456\n\
+                 Chat-Group-Name: another name updat\n\
+                 Chat-Group-Name-Changed: another name update\n\
+                 Date: Sun, 22 Mar 2021 03:00:00 +0000\n\
+                 \n\
+                 5th message\n",
+            false,
+        )
+        .await?;
+        let chat = Chat::load_from_db(&t, chat.id).await?;
+        assert_eq!(chat.name, "another name updat");
+
         Ok(())
     }
 }