fix test, and cleanup according profile-image API

python/src/deltachat/account.py

@@ -483,6 +483,10 @@ class EventLogger:
     def set_timeout(self, timeout):
         self._timeout = timeout
 
+    def consume_events(self, check_error=True):
+        while not self._event_queue.empty():
+            self.get()
+
     def get(self, timeout=None, check_error=True):
         timeout = timeout or self._timeout
         ev = self._event_queue.get(timeout=timeout)

python/tests/test_account.py

@@ -147,7 +147,7 @@ class TestOfflineChat:
         qr = chat.get_join_qr()
         assert ac2.check_qr(qr).is_ask_verifygroup
 
-    def test_get_set_profile_image(self, ac1, data):
+    def test_get_set_profile_image_simple(self, ac1, data):
         chat = ac1.create_group_chat(name="title1")
         p = data.get_path("d.png")
         chat.set_profile_image(p)
@@ -614,16 +614,22 @@ class TestOnlineAccount:
         ac1._evlogger.get_matching("DC_EVENT_CHAT_MODIFIED")
         assert not chat.is_promoted()
 
-        lp.sec("add ac2 to unpromoted group chat")
+        # XXX first promote the chat before setting group image
-        c2 = ac1.create_contact(email=ac2.get_config("addr"))
+        # because DC does not honor it before promotion happened
-        contact1 = chat.add_contact(c2)
+        # unless you add yet another member, see step below.
-        assert not chat.is_promoted()
 
-        lp.sec("ac2: add ac1 per chat")
+        chat.send_text("ac1: initial message to promote chat (workaround)")
+        assert chat.is_promoted()
+
+        lp.sec("ac2: add ac1 to a chat so the message does not land in DEADDROP")
         c1 = ac2.create_contact(email=ac1.get_config("addr"))
         ac2.create_chat_by_contact(c1)
         ev = ac2._evlogger.get_matching("DC_EVENT_MSGS_CHANGED")
 
+        lp.sec("ac1: add ac2 to promoted group chat")
+        c2 = ac1.create_contact(email=ac2.get_config("addr"))
+        contact1 = chat.add_contact(c2)
+
         lp.sec("ac1: send a first message to ac2")
         chat.send_text("hi")
         assert chat.is_promoted()
@@ -631,11 +637,26 @@ class TestOnlineAccount:
         lp.sec("ac2: wait for receiving message from ac1")
         ev = ac2._evlogger.get_matching("DC_EVENT_INCOMING_MSG")
         msg_in = ac2.get_message_by_id(ev[2])
-        assert msg_in.text == "hi"
         assert not msg_in.chat.is_deaddrop()
 
-        lp.sec("ac2: create proper chat and read profile image")
+        lp.sec("ac2: create chat and read profile image")
         chat2 = ac2.create_chat_by_message(msg_in)
         p2 = chat2.get_profile_image()
         assert p2 is not None
         assert open(p2, "rb").read() == open(p, "rb").read()
+
+        ac2._evlogger.consume_events()
+        ac1._evlogger.consume_events()
+        lp.sec("ac2: delete profile image from chat")
+        chat2.remove_profile_image()
+        ev = ac1._evlogger.get_matching("DC_EVENT_INCOMING_MSG")
+        assert ev[1] == chat.id
+        chat1b = ac1.create_chat_by_message(ev[2])
+        assert chat1b.get_profile_image() is None
+        assert chat.get_profile_image() is None
+
+
+
+
+
+

src/chat.rs

@@ -72,15 +72,15 @@ impl<'a> Chat<'a> {
             },
             Ok(mut chat) => {
                 match chat.id {
-                    1 => {
+                    DC_CHAT_ID_DEADDROP => {
                         chat.name = chat.context.stock_str(StockMessage::DeadDrop).into();
                     }
-                    6 => {
+                    DC_CHAT_ID_ARCHIVED_LINK => {
                         let tempname = chat.context.stock_str(StockMessage::ArchivedChats);
                         let cnt = dc_get_archived_cnt(chat.context);
                         chat.name = format!("{} ({})", tempname, cnt);
                     }
-                    5 => {
+                    DC_CHAT_ID_STARRED => {
                         chat.name = chat.context.stock_str(StockMessage::StarredMsgs).into();
                     }
                     _ => {
@@ -187,10 +187,15 @@ impl<'a> Chat<'a> {
             .ok()
     }
 
-    pub unsafe fn get_profile_image(&self) -> Option<String> {
+    pub fn get_profile_image(&self) -> Option<String> {
         if let Some(image_rel) = self.param.get(Param::ProfileImage) {
             if !image_rel.is_empty() {
-                return Some(to_string(dc_get_abs_path(self.context, image_rel)));
+                return Some(
+                    dc_get_abs_path_safe(self.context, image_rel)
+                        .to_str()
+                        .unwrap()
+                        .to_string(),
+                );
             }
         } else if self.typ == Chattype::Single {
             let contacts = get_chat_contacts(self.context, self.id);
@@ -229,6 +234,10 @@ impl<'a> Chat<'a> {
         self.param.get_int(Param::Unpromoted).unwrap_or_default() == 1
     }
 
+    pub fn is_promoted(&self) -> bool {
+        !self.is_unpromoted()
+    }
+
     pub fn is_verified(&self) -> bool {
         (self.typ == Chattype::VerifiedGroup)
     }
@@ -1501,7 +1510,7 @@ pub unsafe fn remove_contact_from_chat(
             } else {
                 /* we should respect this - whatever we send to the group, it gets discarded anyway! */
                 if let Ok(contact) = Contact::get_by_id(context, contact_id) {
-                    if chat.param.get_int(Param::Unpromoted).unwrap_or_default() == 0 {
+                    if chat.is_promoted() {
                         msg.type_0 = Viewtype::Text;
                         if contact.id == DC_CONTACT_ID_SELF {
                             set_group_explicitly_left(context, chat.grpid).unwrap();
@@ -1609,7 +1618,7 @@ pub unsafe fn set_chat_name(
             )
             .is_ok()
             {
-                if chat.param.get_int(Param::Unpromoted).unwrap_or_default() == 0 {
+                if chat.is_promoted() {
                     msg.type_0 = Viewtype::Text;
                     msg.text = Some(context.stock_system_msg(
                         StockMessage::MsgGrpName,
@@ -1646,7 +1655,7 @@ pub unsafe fn set_chat_name(
 }
 
 #[allow(non_snake_case)]
-pub unsafe fn set_chat_profile_image(
+pub fn set_chat_profile_image(
     context: &Context,
     chat_id: u32,
     new_image: impl AsRef<str>,
@@ -1656,7 +1665,7 @@ pub unsafe fn set_chat_profile_image(
     let mut chat = Chat::load_from_db(context, chat_id)?;
 
     if real_group_exists(context, chat_id) {
-        if !(is_contact_in_chat(context, chat_id, 1i32 as u32) == 1i32) {
+        if !(is_contact_in_chat(context, chat_id, DC_CONTACT_ID_SELF) == 1i32) {
             log_event!(
                 context,
                 Event::ERROR_SELF_NOT_IN_GROUP,
@@ -1678,17 +1687,16 @@ pub unsafe fn set_chat_profile_image(
 
         chat.param.set(Param::ProfileImage, &new_image_rel);
         if chat.update_param().is_ok() {
-            info!(context, 0, "after update_param");
+            if chat.is_promoted() {
-            if chat.param.get_int(Param::Unpromoted).unwrap_or_default() == 0 {
+                let mut msg = unsafe { dc_msg_new_untyped(context) };
-                let mut msg = dc_msg_new_untyped(context);
-                info!(context, 0, "setting params for groupimage change");
                 msg.param.set_int(Param::Cmd, DC_CMD_GROUPIMAGE_CHANGED);
-                msg.param.set(Param::Arg, &new_image_rel);
                 msg.type_0 = Viewtype::Text;
                 msg.text = Some(context.stock_system_msg(
-                    if new_image_rel.is_empty() {
+                    if new_image_rel == "" {
+                        msg.param.remove(Param::Arg);
                         StockMessage::MsgGrpImgDeleted
                     } else {
+                        msg.param.set(Param::Arg, &new_image_rel);
                         StockMessage::MsgGrpImgChanged
                     },
                     "",

src/constants.rs

@@ -77,7 +77,7 @@ pub const DC_CHAT_ID_TRASH: u32 = 3;
 /// a message is just in creation but not yet assigned to a chat (eg. we may need the message ID to set up blobs; this avoids unready message to be sent and shown)
 const DC_CHAT_ID_MSGS_IN_CREATION: u32 = 4;
 /// virtual chat showing all messages flagged with msgs.starred=2
-const DC_CHAT_ID_STARRED: u32 = 5;
+pub const DC_CHAT_ID_STARRED: u32 = 5;
 /// only an indicator in a chatlist
 pub const DC_CHAT_ID_ARCHIVED_LINK: u32 = 6;
 /// only an indicator in a chatlist

src/dc_mimefactory.rs

@@ -532,10 +532,6 @@ pub unsafe fn dc_mimefactory_render(factory: &mut dc_mimefactory_t) -> libc::c_i
 
             /* build header etc. */
             let command = factory.msg.param.get_int(Param::Cmd).unwrap_or_default();
-            info!(
-                factory.context,
-                0, "render_message found command {}", command
-            );
             if chat.typ == Chattype::Group || chat.typ == Chattype::VerifiedGroup {
                 mailimf_fields_add(
                     imf_fields,
@@ -729,9 +725,8 @@ pub unsafe fn dc_mimefactory_render(factory: &mut dc_mimefactory_t) -> libc::c_i
                     }
                 }
             }
-            info!(factory.context, 0, "grpimage {:?}", grpimage);
             if let Some(grpimage) = grpimage {
-                info!(factory.context, 0, "setting group image");
+                info!(factory.context, 0, "setting group image '{}'", grpimage);
                 let mut meta = dc_msg_new_untyped(factory.context);
                 meta.type_0 = Viewtype::Image;
                 meta.param.set(Param::File, grpimage);
@@ -1089,7 +1084,7 @@ unsafe fn get_subject(
     } else {
         b"\x00" as *const u8 as *const libc::c_char
     };
-    if msg.param.get_int(Param::Cmd).unwrap_or_default() == 6 {
+    if msg.param.get_int(Param::Cmd).unwrap_or_default() == DC_CMD_AUTOCRYPT_SETUP_MESSAGE {
         ret = context.stock_str(StockMessage::AcSetupMsgSubject).strdup()
     } else if chat.typ == Chattype::Group || chat.typ == Chattype::VerifiedGroup {
         ret = format!(

src/dc_receive_imf.rs

@@ -1049,7 +1049,7 @@ unsafe fn create_or_lookup_group(
     // pointer somewhere into mime_parser, must not be freed
     let mut X_MrAddToGrp = std::ptr::null_mut();
     let mut X_MrGrpNameChanged = 0;
-    let mut X_MrGrpImageChanged = std::ptr::null();
+    let mut X_MrGrpImageChanged = "".to_string();
     let mut better_msg: String = From::from("");
     let mut failure_reason = std::ptr::null_mut();
 
@@ -1172,7 +1172,8 @@ unsafe fn create_or_lookup_group(
             let optional_field =
                 dc_mimeparser_lookup_optional_field(mime_parser, "Chat-Group-Image");
             if !optional_field.is_null() {
-                X_MrGrpImageChanged = (*optional_field).fld_value
+                // fld_value is a pointer somewhere into mime_parser, must not be freed
+                X_MrGrpImageChanged = as_str((*optional_field).fld_value).to_string();
             }
             better_msg = context.stock_system_msg(
                 StockMessage::MsgAddMember,
@@ -1196,14 +1197,11 @@ unsafe fn create_or_lookup_group(
                 let optional_field =
                     dc_mimeparser_lookup_optional_field(mime_parser, "Chat-Group-Image");
                 if !optional_field.is_null() {
-                    X_MrGrpImageChanged = (*optional_field).fld_value;
+                    // fld_value is a pointer somewhere into mime_parser, must not be freed
+                    X_MrGrpImageChanged = as_str((*optional_field).fld_value).to_string();
                     mime_parser.is_system_message = DC_CMD_GROUPIMAGE_CHANGED;
                     better_msg = context.stock_system_msg(
-                        if strcmp(
+                        if X_MrGrpImageChanged == "0" {
-                            X_MrGrpImageChanged,
-                            b"0\x00" as *const u8 as *const libc::c_char,
-                        ) == 0
-                        {
                             StockMessage::MsgGrpImgDeleted
                         } else {
                             StockMessage::MsgGrpImgChanged
@@ -1315,7 +1313,6 @@ unsafe fn create_or_lookup_group(
     }
 
     // execute group commands
-    info!(context, 0, "before exec group commands");
     if !X_MrAddToGrp.is_null() || !X_MrRemoveFromGrp.is_null() {
         recreate_member_list = 1;
     } else if 0 != X_MrGrpNameChanged && !grpname.is_null() && strlen(grpname) < 200 {
@@ -1331,57 +1328,39 @@ unsafe fn create_or_lookup_group(
             context.call_cb(Event::CHAT_MODIFIED, chat_id as uintptr_t, 0);
         }
     }
-    if !X_MrGrpImageChanged.is_null() {
+    if !X_MrGrpImageChanged.is_empty() {
         info!(
             context,
-            0,
+            0, "grp-image-change {} chat {}", X_MrGrpImageChanged, chat_id
-            "handling group image changed {} for chat {}",
-            as_str(X_MrGrpImageChanged),
-            chat_id
         );
-        let mut ok = 0;
+        let mut changed = false;
-        let mut grpimage = ptr::null_mut();
+        let mut grpimage = "".to_string();
-        if strcmp(
+        if X_MrGrpImageChanged == "0" {
-            X_MrGrpImageChanged,
+            changed = true;
-            b"0\x00" as *const u8 as *const libc::c_char,
-        ) == 0
-        {
-            ok = 1
         } else {
             for part in &mut mime_parser.parts {
                 if part.type_0 == Viewtype::Image {
                     grpimage = part
                         .param
                         .get(Param::File)
-                        .map(|s| s.strdup())
+                        .map(|s| s.to_string())
-                        .unwrap_or_else(|| std::ptr::null_mut());
+                        .unwrap_or_else(|| "".to_string());
                     info!(context, 0, "found image {:?}", grpimage);
-                    ok = 1
+                    changed = true;
                 }
             }
         }
-        if 0 != ok {
+        if changed {
-            info!(
+            info!(context, 0, "New group image set to '{}'.", grpimage);
-                context,
-                0,
-                "New group image set to {}.",
-                if !grpimage.is_null() {
-                    "DELETED".to_string()
-                } else {
-                    to_string(grpimage)
-                },
-            );
             if let Ok(mut chat) = Chat::load_from_db(context, chat_id) {
-                if grpimage.is_null() {
+                if grpimage.is_empty() {
                     chat.param.remove(Param::ProfileImage);
                 } else {
-                    chat.param.set(Param::ProfileImage, as_str(grpimage));
+                    chat.param.set(Param::ProfileImage, grpimage);
                 }
                 chat.update_param().unwrap();
                 send_EVENT_CHAT_MODIFIED = 1;
             }
-
-            free(grpimage as *mut libc::c_void);
         }
     }