Limit cases when contact's status and avatar are updated (#7002)

src/chat.rs

@@ -4025,12 +4025,12 @@ pub(crate) async fn add_contact_to_chat_ex(
     Ok(true)
 }
 
-/// Returns true if an avatar should be attached in the given chat.
+/// Returns whether profile data should be attached when sending to the given chat.
 ///
-/// This function does not check if the avatar is set.
+/// This function does not check if the avatar/status is set.
 /// If avatar is not set and this function returns `true`,
 /// a `Chat-User-Avatar: 0` header should be sent to reset the avatar.
-pub(crate) async fn shall_attach_selfavatar(context: &Context, chat_id: ChatId) -> Result<bool> {
+pub(crate) async fn should_attach_profile(context: &Context, chat_id: ChatId) -> Result<bool> {
     let timestamp_some_days_ago = time() - DC_RESEND_USER_AVATAR_DAYS * 24 * 60 * 60;
     let needs_attach = context
         .sql
@@ -4045,8 +4045,8 @@ pub(crate) async fn shall_attach_selfavatar(context: &Context, chat_id: ChatId)
                 let mut needs_attach = false;
                 for row in rows {
                     let row = row?;
-                    let selfavatar_sent = row?;
-                    if selfavatar_sent < timestamp_some_days_ago {
+                    let profile_sent = row?;
+                    if profile_sent < timestamp_some_days_ago {
                         needs_attach = true;
                     }
                 }

src/chat/chat_tests.rs

@@ -1540,23 +1540,23 @@ async fn test_create_same_chat_twice() {
 }
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
-async fn test_shall_attach_selfavatar() -> Result<()> {
+async fn test_should_attach_profile() -> Result<()> {
     let mut tcm = TestContextManager::new();
     let alice = &tcm.alice().await;
     let bob = &tcm.bob().await;
 
     let chat_id = create_group_chat(alice, ProtectionStatus::Unprotected, "foo").await?;
-    assert!(!shall_attach_selfavatar(alice, chat_id).await?);
+    assert!(!should_attach_profile(alice, chat_id).await?);
 
     let contact_id = alice.add_or_lookup_contact_id(bob).await;
     add_contact_to_chat(alice, chat_id, contact_id).await?;
-    assert!(shall_attach_selfavatar(alice, chat_id).await?);
+    assert!(should_attach_profile(alice, chat_id).await?);
 
     chat_id.set_selfavatar_timestamp(alice, time()).await?;
-    assert!(!shall_attach_selfavatar(alice, chat_id).await?);
+    assert!(!should_attach_profile(alice, chat_id).await?);
 
     alice.set_config(Config::Selfavatar, None).await?; // setting to None also forces re-sending
-    assert!(shall_attach_selfavatar(alice, chat_id).await?);
+    assert!(should_attach_profile(alice, chat_id).await?);
     Ok(())
 }
 
@@ -1580,7 +1580,7 @@ async fn test_profile_data_on_group_leave() -> Result<()> {
     tokio::fs::write(&file, bytes).await?;
     t.set_config(Config::Selfavatar, Some(file.to_str().unwrap()))
         .await?;
-    assert!(shall_attach_selfavatar(t, chat_id).await?);
+    assert!(should_attach_profile(t, chat_id).await?);
 
     remove_contact_from_chat(t, chat_id, ContactId::SELF).await?;
     let sent_msg = t.pop_sent_msg().await;

src/config.rs

@@ -759,6 +759,14 @@ impl Context {
         let better_value;
 
         match key {
+            Config::Selfstatus => {
+                // Ensure that future versions send the self-status that after upgrade. Currently we
+                // send it in every appropriate message.
+                self.sql
+                    .execute("UPDATE contacts SET selfavatar_sent=0", ())
+                    .await?;
+                self.sql.set_raw_config(key.as_ref(), value).await?;
+            }
             Config::Selfavatar => {
                 self.sql
                     .execute("UPDATE contacts SET selfavatar_sent=0;", ())

src/config/config_tests.rs

@@ -278,7 +278,6 @@ async fn test_sync() -> Result<()> {
     Ok(())
 }
 
-/// Sync message mustn't be sent if self-{status,avatar} is changed by a self-sent message.
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_no_sync_on_self_sent_msg() -> Result<()> {
     let mut tcm = TestContextManager::new();
@@ -288,7 +287,7 @@ async fn test_no_sync_on_self_sent_msg() -> Result<()> {
         a.set_config_bool(Config::SyncMsgs, true).await?;
     }
 
-    let status = "Synced via usual message";
+    let status = "Sent via usual message";
     alice0.set_config(Config::Selfstatus, Some(status)).await?;
     alice0.send_sync_msg().await?;
     alice0.pop_sent_sync_msg().await;
@@ -297,7 +296,7 @@ async fn test_no_sync_on_self_sent_msg() -> Result<()> {
     tcm.send_recv(alice0, alice1, "hi Alice!").await;
     assert_eq!(
         alice1.get_config(Config::Selfstatus).await?,
-        Some(status.to_string())
+        Some(status1.to_string())
     );
     sync(alice1, alice0).await;
     assert_eq!(
@@ -328,7 +327,7 @@ async fn test_no_sync_on_self_sent_msg() -> Result<()> {
         alice1
             .get_config(Config::Selfavatar)
             .await?
-            .filter(|path| path.ends_with(".png"))
+            .filter(|path| path.ends_with(".jpg"))
             .is_some()
     );
     sync(alice1, alice0).await;

src/contact.rs

@@ -383,19 +383,15 @@ async fn import_vcard_contact(context: &Context, contact: &VcardContact) -> Resu
         None => None,
     };
     if let Some(path) = path {
-        // Currently this value doesn't matter as we don't import the contact of self.
-        let was_encrypted = false;
-        if let Err(e) =
-            set_profile_image(context, id, &AvatarAction::Change(path), was_encrypted).await
-        {
+        if let Err(e) = set_profile_image(context, id, &AvatarAction::Change(path)).await {
             warn!(
                 context,
                 "import_vcard_contact: Could not set avatar for {}: {e:#}.", contact.addr
             );
         }
     }
     if let Some(biography) = &contact.biography {
-        if let Err(e) = set_status(context, id, biography.to_owned(), false, false).await {
+        if let Err(e) = set_status(context, id, biography.to_owned()).await {
             warn!(
                 context,
                 "import_vcard_contact: Could not set biography for {}: {e:#}.", contact.addr
@@ -1835,39 +1831,29 @@ WHERE type=? AND id IN (
 /// The given profile image is expected to be already in the blob directory
 /// as profile images can be set only by receiving messages, this should be always the case, however.
 ///
-/// For contact SELF, the image is not saved in the contact-database but as Config::Selfavatar;
-/// this typically happens if we see message with our own profile image.
+/// For contact SELF, the image is not saved in the contact-database but as Config::Selfavatar.
 pub(crate) async fn set_profile_image(
     context: &Context,
     contact_id: ContactId,
     profile_image: &AvatarAction,
-    was_encrypted: bool,
 ) -> Result<()> {
     let mut contact = Contact::get_by_id(context, contact_id).await?;
     let changed = match profile_image {
         AvatarAction::Change(profile_image) => {
             if contact_id == ContactId::SELF {
-                if was_encrypted {
-                    context
-                        .set_config_ex(Nosync, Config::Selfavatar, Some(profile_image))
-                        .await?;
-                } else {
-                    info!(context, "Do not use unencrypted selfavatar.");
-                }
+                context
+                    .set_config_ex(Nosync, Config::Selfavatar, Some(profile_image))
+                    .await?;
             } else {
                 contact.param.set(Param::ProfileImage, profile_image);
             }
             true
         }
         AvatarAction::Delete => {
             if contact_id == ContactId::SELF {
-                if was_encrypted {
-                    context
-                        .set_config_ex(Nosync, Config::Selfavatar, None)
-                        .await?;
-                } else {
-                    info!(context, "Do not use unencrypted selfavatar deletion.");
-                }
+                context
+                    .set_config_ex(Nosync, Config::Selfavatar, None)
+                    .await?;
             } else {
                 contact.param.remove(Param::ProfileImage);
             }
@@ -1884,22 +1870,16 @@ pub(crate) async fn set_profile_image(
 
 /// Sets contact status.
 ///
-/// For contact SELF, the status is not saved in the contact table, but as Config::Selfstatus.  This
-/// is only done if message is sent from Delta Chat and it is encrypted, to synchronize signature
-/// between Delta Chat devices.
+/// For contact SELF, the status is not saved in the contact table, but as Config::Selfstatus.
 pub(crate) async fn set_status(
     context: &Context,
     contact_id: ContactId,
     status: String,
-    encrypted: bool,
-    has_chat_version: bool,
 ) -> Result<()> {
     if contact_id == ContactId::SELF {
-        if encrypted && has_chat_version {
-            context
-                .set_config_ex(Nosync, Config::Selfstatus, Some(&status))
-                .await?;
-        }
+        context
+            .set_config_ex(Nosync, Config::Selfstatus, Some(&status))
+            .await?;
     } else {
         let mut contact = Contact::get_by_id(context, contact_id).await?;
 

src/contact/contact_tests.rs

@@ -4,7 +4,7 @@ use super::*;
 use crate::chat::{Chat, get_chat_contacts, send_text_msg};
 use crate::chatlist::Chatlist;
 use crate::receive_imf::receive_imf;
-use crate::test_utils::{self, TestContext, TestContextManager, TimeShiftFalsePositiveNote};
+use crate::test_utils::{self, TestContext, TestContextManager, TimeShiftFalsePositiveNote, sync};
 
 #[test]
 fn test_contact_id_values() {
@@ -831,8 +831,7 @@ CCCB 5AA9 F6E1 141C 9431
     Ok(())
 }
 
-/// Tests that status is synchronized when sending encrypted BCC-self messages and not
-/// synchronized when the message is not encrypted.
+/// Tests that self-status is not synchronized from outgoing messages.
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_synchronize_status() -> Result<()> {
     let mut tcm = TestContextManager::new();
@@ -851,39 +850,22 @@ async fn test_synchronize_status() -> Result<()> {
         .await?;
     let chat = alice1.create_email_chat(bob).await;
 
-    // Alice sends a message to Bob from the first device.
+    // Alice sends an unencrypted message to Bob from the first device.
     send_text_msg(alice1, chat.id, "Hello".to_string()).await?;
     let sent_msg = alice1.pop_sent_msg().await;
 
-    // Message is not encrypted.
-    let message = sent_msg.load_from_db().await;
-    assert!(!message.get_showpadlock());
-
     // Alice's second devices receives a copy of outgoing message.
     alice2.recv_msg(&sent_msg).await;
-
-    // Bob receives message.
-    bob.recv_msg(&sent_msg).await;
-
-    // Message was not encrypted, so status is not copied.
     assert_eq!(alice2.get_config(Config::Selfstatus).await?, default_status);
 
     // Alice sends encrypted message.
     let chat = alice1.create_chat(bob).await;
     send_text_msg(alice1, chat.id, "Hello".to_string()).await?;
     let sent_msg = alice1.pop_sent_msg().await;
 
-    // Second message is encrypted.
-    let message = sent_msg.load_from_db().await;
-    assert!(message.get_showpadlock());
-
     // Alice's second devices receives a copy of second outgoing message.
     alice2.recv_msg(&sent_msg).await;
-
-    assert_eq!(
-        alice2.get_config(Config::Selfstatus).await?,
-        Some("New status".to_string())
-    );
+    assert_eq!(alice2.get_config(Config::Selfstatus).await?, default_status);
 
     Ok(())
 }
@@ -896,9 +878,9 @@ async fn test_selfavatar_changed_event() -> Result<()> {
     // Alice has two devices.
     let alice1 = &tcm.alice().await;
     let alice2 = &tcm.alice().await;
-
-    // Bob has one device.
-    let bob = &tcm.bob().await;
+    for a in [alice1, alice2] {
+        a.set_config_bool(Config::SyncMsgs, true).await?;
+    }
 
     assert_eq!(alice1.get_config(Config::Selfavatar).await?, None);
 
@@ -914,17 +896,7 @@ async fn test_selfavatar_changed_event() -> Result<()> {
         .get_matching(|e| matches!(e, EventType::SelfavatarChanged))
         .await;
 
-    // Alice sends a message.
-    let alice1_chat_id = alice1.create_chat(bob).await.id;
-    send_text_msg(alice1, alice1_chat_id, "Hello".to_string()).await?;
-    let sent_msg = alice1.pop_sent_msg().await;
-
-    // The message is encrypted.
-    let message = sent_msg.load_from_db().await;
-    assert!(message.get_showpadlock());
-
-    // Alice's second device receives a copy of the outgoing message.
-    alice2.recv_msg(&sent_msg).await;
+    sync(alice1, alice2).await;
 
     // Alice's second device applies the selfavatar.
     assert!(alice2.get_config(Config::Selfavatar).await?.is_some());

src/headerdef.rs

@@ -60,7 +60,12 @@ pub enum HeaderDef {
     ChatGroupNameTimestamp,
     ChatVerified,
     ChatGroupAvatar,
+
+    /// If present, contact's avatar and status should be applied from the message.
+    /// "Chat-User-Avatar: 0" means that the contact has no avatar. Contact's status is transferred
+    /// in the message footer.
     ChatUserAvatar,
+
     ChatVoiceMessage,
     ChatGroupMemberRemoved,
     ChatGroupMemberAdded,