Refactor mimefactory to have separate To field and recipient list

Author: link2xt

src/mimefactory.rs

@@ -66,8 +66,26 @@ pub struct MimeFactory {
 
     selfstatus: String,
 
-    /// Vector of pairs of recipient name and address
-    recipients: Vec<(String, String)>,
+    /// Vector of actual recipient addresses.
+    ///
+    /// This is the list of addresses the message should be sent to.
+    /// It is not the same as the `To` header,
+    /// because in case of "member removed" message
+    /// removed member is in the recipient list,
+    /// but not in the `To` header.
+    /// In case of broadcast lists there are multiple recipients,
+    /// but the `To` header has no members.
+    ///
+    /// If `bcc_self` configuration is enabled,
+    /// this list will be extended with own address later,
+    /// but `MimeFactory` is not responsible for this.
+    recipients: Vec<String>,
+
+    /// Vector of pairs of recipient name and address that goes into the `To` field.
+    ///
+    /// The list of actual message recipient addresses may be different,
+    /// e.g. if members are hidden for broadcast lists.
+    to: Vec<(String, String)>,
 
     /// Vector of pairs of past group member name and addresses.
     past_members: Vec<(String, String)>,
@@ -138,6 +156,7 @@ impl MimeFactory {
     pub async fn from_msg(context: &Context, msg: Message) -> Result<MimeFactory> {
         let chat = Chat::load_from_db(context, msg.chat_id).await?;
         let attach_profile_data = Self::should_attach_profile_data(&msg);
+        let undisclosed_recipients = chat.typ == Chattype::Broadcast;
 
         let from_addr = context.get_primary_self_addr().await?;
         let config_displayname = context
@@ -155,23 +174,32 @@ impl MimeFactory {
                 (name, None)
             };
 
-        let mut recipients = Vec::with_capacity(5);
+        let mut recipients = Vec::new();
+        let mut to = Vec::new();
         let mut past_members = Vec::new();
         let mut member_timestamps = Vec::new();
         let mut recipient_ids = HashSet::new();
         let mut req_mdn = false;
 
         if chat.is_self_talk() {
             if msg.param.get_cmd() == SystemMessage::AutocryptSetupMessage {
-                recipients.push((from_displayname.to_string(), from_addr.to_string()));
+                recipients.push(from_addr.to_string());
+                to.push((from_displayname.to_string(), from_addr.to_string()));
             }
         } else if chat.is_mailing_list() {
             let list_post = chat
                 .param
                 .get(Param::ListPost)
                 .context("Can't write to mailinglist without ListPost param")?;
-            recipients.push(("".to_string(), list_post.to_string()));
+            to.push(("".to_string(), list_post.to_string()));
+            recipients.push(list_post.to_string());
         } else {
+            let email_to_remove = if msg.param.get_cmd() == SystemMessage::MemberRemovedFromGroup {
+                msg.param.get(Param::Arg)
+            } else {
+                None
+            };
+
             context
                 .sql
                 .query_map(
@@ -196,17 +224,37 @@ impl MimeFactory {
                                 false => "".to_string(),
                             };
                             if add_timestamp >= remove_timestamp {
-                                if !recipients_contain_addr(&recipients, &addr) {
-                                    recipients.push((name, addr));
-                                    member_timestamps.push(add_timestamp);
+                                if !recipients_contain_addr(&to, &addr) {
+                                    recipients.push(addr.clone());
+                                    if !undisclosed_recipients {
+                                        if let Some(email_to_remove) = email_to_remove {
+                                            if email_to_remove != addr {
+                                                to.push((name, addr));
+                                                member_timestamps.push(add_timestamp);
+                                            }
+                                        } else {
+                                            to.push((name, addr));
+                                            member_timestamps.push(add_timestamp);
+                                        }
+                                    }
                                 }
                                 recipient_ids.insert(id);
                             } else {
                                 // Row is a tombstone,
                                 // member is not actually part of the group.
                                 if !recipients_contain_addr(&past_members, &addr) {
-                                    past_members.push((name, addr));
-                                    member_timestamps.push(remove_timestamp);
+                                    if let Some(email_to_remove) = email_to_remove {
+                                        if email_to_remove == addr {
+                                            // This is a "member removed" message,
+                                            // we need to notify removed member
+                                            // that it was removed.
+                                            recipients.push(addr.clone());
+                                        }
+                                    }
+                                    if !undisclosed_recipients {
+                                        past_members.push((name, addr));
+                                        member_timestamps.push(remove_timestamp);
+                                    }
                                 }
                             }
                         }
@@ -253,14 +301,15 @@ impl MimeFactory {
 
         debug_assert!(
             member_timestamps.is_empty()
-                || recipients.len() + past_members.len() == member_timestamps.len()
+                || to.len() + past_members.len() == member_timestamps.len()
         );
         let factory = MimeFactory {
             from_addr,
             from_displayname,
             sender_displayname,
             selfstatus,
             recipients,
+            to,
             past_members,
             member_timestamps,
             timestamp: msg.timestamp_sort,
@@ -290,7 +339,8 @@ impl MimeFactory {
             from_displayname: "".to_string(),
             sender_displayname: None,
             selfstatus: "".to_string(),
-            recipients: vec![("".to_string(), contact.get_addr().to_string())],
+            recipients: vec![contact.get_addr().to_string()],
+            to: vec![("".to_string(), contact.get_addr().to_string())],
             past_members: vec![],
             member_timestamps: vec![],
             timestamp,
@@ -316,11 +366,7 @@ impl MimeFactory {
         let self_addr = context.get_primary_self_addr().await?;
 
         let mut res = Vec::new();
-        for (_, addr) in self
-            .recipients
-            .iter()
-            .filter(|(_, addr)| addr != &self_addr)
-        {
+        for addr in self.recipients.iter().filter(|&addr| *addr != self_addr) {
             res.push((Peerstate::from_addr(context, addr).await?, addr.clone()));
         }
 
@@ -508,10 +554,7 @@ impl MimeFactory {
     }
 
     pub fn recipients(&self) -> Vec<String> {
-        self.recipients
-            .iter()
-            .map(|(_, addr)| addr.clone())
-            .collect()
+        self.recipients.clone()
     }
 
     /// Consumes a `MimeFactory` and renders it into a message which is then stored in
@@ -521,56 +564,26 @@ impl MimeFactory {
 
         let from = new_address_with_name(&self.from_displayname, self.from_addr.clone());
 
-        let undisclosed_recipients = match &self.loaded {
-            Loaded::Message { chat, .. } => chat.typ == Chattype::Broadcast,
-            Loaded::Mdn { .. } => false,
-        };
-
         let mut to = Vec::new();
-        let mut past_members = Vec::new(); // Contents of `Chat-Group-Past-Members` header.
-        if !undisclosed_recipients {
-            let email_to_remove = match &self.loaded {
-                Loaded::Message { msg, .. } => {
-                    if msg.param.get_cmd() == SystemMessage::MemberRemovedFromGroup {
-                        msg.param.get(Param::Arg)
-                    } else {
-                        None
-                    }
-                }
-                Loaded::Mdn { .. } => None,
-            };
-
-            for (name, addr) in &self.recipients {
-                if let Some(email_to_remove) = email_to_remove {
-                    if email_to_remove == addr {
-                        if name.is_empty() {
-                            past_members.push(Address::new_mailbox(addr.clone()));
-                        } else {
-                            past_members.push(new_address_with_name(name, addr.clone()));
-                        }
-                        continue;
-                    }
-                }
-
-                if name.is_empty() {
-                    to.push(Address::new_mailbox(addr.clone()));
-                } else {
-                    to.push(new_address_with_name(name, addr.clone()));
-                }
+        for (name, addr) in &self.to {
+            if name.is_empty() {
+                to.push(Address::new_mailbox(addr.clone()));
+            } else {
+                to.push(new_address_with_name(name, addr.clone()));
             }
+        }
 
-            for (name, addr) in &self.past_members {
-                if name.is_empty() {
-                    past_members.push(Address::new_mailbox(addr.clone()));
-                } else {
-                    past_members.push(new_address_with_name(name, addr.clone()));
-                }
+        let mut past_members = Vec::new(); // Contents of `Chat-Group-Past-Members` header.
+        for (name, addr) in &self.past_members {
+            if name.is_empty() {
+                past_members.push(Address::new_mailbox(addr.clone()));
+            } else {
+                past_members.push(new_address_with_name(name, addr.clone()));
             }
         }
 
         debug_assert!(
-            undisclosed_recipients
-                || self.member_timestamps.is_empty()
+            self.member_timestamps.is_empty()
                 || to.len() + past_members.len() == self.member_timestamps.len()
         );
         if to.is_empty() {
@@ -597,7 +610,7 @@ impl MimeFactory {
             );
         }
 
-        if !undisclosed_recipients && !self.member_timestamps.is_empty() {
+        if !self.member_timestamps.is_empty() {
             headers.push(
                 Header::new_with_value(
                     "Chat-Group-Member-Timestamps".into(),