email/address: Refactor References struct

Signed-off-by: Manos Pitsidianakis <manos@pitsidianak.is>

meli/src/mail/view/envelope.rs

@@ -841,17 +841,15 @@ impl Component for EnvelopeView {
                     )
                 );
                 if self.view_settings.expand_headers {
-                    if let Some(val) = envelope.in_reply_to_display() {
+                    if let Some(val) = envelope.in_reply_to() {
                         print_header!(
-                            (HeaderName::IN_REPLY_TO, val),
+                            (
+                                HeaderName::IN_REPLY_TO,
+                                melib::MessageID::display_slice(val.refs(), Some(" "))
+                            ),
                             (
                                 HeaderName::REFERENCES,
-                                envelope
-                                    .references()
-                                    .iter()
-                                    .map(std::string::ToString::to_string)
-                                    .collect::<Vec<String>>()
-                                    .join(", ")
+                                melib::MessageID::display_slice(envelope.references(), Some(" "))
                             )
                         );
                     }

meli/src/sqlite3.rs

@@ -219,8 +219,8 @@ impl AccountCache {
                         envelope.subject().into_owned().trim_end_matches('\u{0}'),
                         envelope.message_id().to_string(),
                         envelope
-                            .in_reply_to_display()
-                            .map(|f| f.to_string())
+                            .in_reply_to()
+                            .map(|f| melib::MessageID::display_slice(f.refs(), Some(" ")))
                             .unwrap_or_default(),
                         envelope.field_references_to_string(),
                         i64::from(envelope.flags().bits()),
@@ -367,8 +367,11 @@ impl AccountCache {
                                     e.field_bcc_to_string(),
                                     e.subject().into_owned().trim_end_matches('\u{0}'),
                                     e.message_id().to_string(),
-                                    e.in_reply_to_display()
-                                        .map(|f| f.to_string())
+                                    e.in_reply_to()
+                                        .map(|f| melib::MessageID::display_slice(
+                                            f.refs(),
+                                            Some(" ")
+                                        ))
                                         .unwrap_or_default(),
                                     e.field_references_to_string(),
                                     i64::from(e.flags().bits()),

melib/src/email.rs

@@ -266,22 +266,19 @@ crate::declare_u64_hash!(EnvelopeHash);
 /// bytes into an `Attachment` object.
 #[derive(Clone, Deserialize, Serialize)]
 pub struct Envelope {
-    // ----- IMAP4rev1 -----
     pub date: String,
     pub subject: Option<String>,
     pub from: SmallVec<[Address; 1]>,
-    // pub sender
-    // pub reply_to
     pub to: SmallVec<[Address; 1]>,
     pub cc: SmallVec<[Address; 1]>,
     pub bcc: Vec<Address>,
-    pub in_reply_to: Option<MessageID>,
+    pub in_reply_to: Option<References>,
+    pub references: Option<References>,
     pub message_id: MessageID,
+    pub other_headers: HeaderMap,
     // ----- Other -----
     pub hash: EnvelopeHash,
     pub timestamp: UnixTimestamp,
-    pub references: Option<References>,
-    pub other_headers: HeaderMap,
     pub thread: ThreadNodeHash,
     pub flags: Flag,
     pub has_attachments: bool,
@@ -461,8 +458,15 @@ impl Envelope {
          *
          * if self.message_id.is_none()  ...
          */
-        if let Some(x) = self.in_reply_to.clone() {
-            self.push_references(x);
+        if let Some(ref x) = self.in_reply_to {
+            match self.references {
+                Some(ref mut s) => {
+                    s.extend(x.refs());
+                }
+                None => {
+                    self.references = Some(x.clone());
+                }
+            }
         }
         if let Ok(d) = parser::dates::rfc5322_date(self.date.as_bytes()) {
             self.set_datetime(d);
@@ -471,17 +475,8 @@ impl Envelope {
             let hash = self.hash;
             self.set_message_id(format!("<{:x}>", hash.0).as_bytes());
         }
-        if self.references.is_some() {
-            if let Some(pos) = self
-                .references
-                .as_ref()
-                .map(|r| &r.refs)
-                .unwrap()
-                .iter()
-                .position(|r| r == &self.message_id)
-            {
-                self.references.as_mut().unwrap().refs.remove(pos);
-            }
+        if let Some(ref mut r) = self.references {
+            r.remove(&self.message_id);
         }
 
         Ok(())
@@ -624,22 +619,13 @@ impl Envelope {
         }
     }
 
-    pub fn in_reply_to(&self) -> Option<&MessageID> {
-        self.in_reply_to
+    pub fn in_reply_to(&'_ self) -> Option<Cow<'_, References>> {
+        self.in_reply_to.as_ref().map(Cow::Borrowed).or_else(|| {
+            self.references
                 .as_ref()
-            .or_else(|| self.references.as_ref().and_then(|r| r.refs.last()))
-    }
-
-    pub fn in_reply_to_display(&self) -> Option<Cow<str>> {
-        self.in_reply_to
-            .as_ref()
-            .map(|m| String::from_utf8_lossy(m.val()))
-    }
-
-    pub fn in_reply_to_raw(&self) -> Option<Cow<str>> {
-        self.in_reply_to
-            .as_ref()
-            .map(|m| String::from_utf8_lossy(m.raw()))
+                .and_then(|r| r.refs().last())
+                .and_then(|msgid| Some(Cow::Owned(References::new(vec![msgid.clone()])?)))
+        })
     }
 
     pub fn message_id(&self) -> &MessageID {
@@ -673,23 +659,35 @@ impl Envelope {
     }
 
     pub fn set_in_reply_to(&mut self, new_val: &[u8]) -> &mut Self {
-        // [ref:FIXME]: msg_id_list
-        let new_val = new_val.trim();
         if !new_val.is_empty() {
-            let val = match parser::address::msg_id(new_val) {
-                Ok(v) => v.1,
-                Err(_) => {
-                    self.in_reply_to = Some(MessageID::new(new_val, new_val));
-                    return self;
-                }
-            };
-            self.in_reply_to = Some(val);
-        } else {
             self.in_reply_to = None;
+            {
+                let parse_result = parser::address::msg_id_list(new_val);
+                if let Ok((_, value)) = parse_result {
+                    for v in value {
+                        self.push_in_reply_to(v);
+                    }
+                }
+            }
+            self.other_headers_mut().insert(
+                HeaderName::IN_REPLY_TO,
+                String::from_utf8_lossy(new_val).to_string(),
+            );
         }
         self
     }
 
+    pub fn push_in_reply_to(&mut self, new_ref: MessageID) {
+        match self.in_reply_to {
+            Some(ref mut s) => {
+                s.extend(std::iter::once(new_ref));
+            }
+            None => {
+                self.in_reply_to = References::new(vec![new_ref]);
+            }
+        }
+    }
+
     pub fn set_subject(&mut self, new_val: Vec<u8>) -> &mut Self {
         let mut new_val = String::from_utf8(new_val)
             .unwrap_or_else(|err| String::from_utf8_lossy(&err.into_bytes()).into());
@@ -719,28 +717,13 @@ impl Envelope {
         self
     }
 
-    pub fn push_references(&mut self, new_ref: MessageID) {
+    pub fn push_references(&mut self, new_refs: &References) {
         match self.references {
             Some(ref mut s) => {
-                if s.refs.contains(&new_ref) {
-                    if s.refs[s.refs.len() - 1] != new_ref {
-                        if let Some(index) = s.refs.iter().position(|x| *x == new_ref) {
-                            s.refs.remove(index);
-                        } else {
-                            panic!();
-                        }
-                    } else {
-                        return;
-                    }
-                }
-                s.refs.push(new_ref);
+                s.extend(new_refs.refs());
             }
             None => {
-                let v = vec![new_ref];
-                self.references = Some(References {
-                    raw: "".into(),
-                    refs: v,
-                });
+                self.references = Some(new_refs.clone());
             }
         }
     }
@@ -752,33 +735,21 @@ impl Envelope {
             {
                 let parse_result = parser::address::msg_id_list(new_val);
                 if let Ok((_, value)) = parse_result {
-                    for v in value {
-                        self.push_references(v);
-                    }
-                }
-            }
-            match self.references {
-                Some(ref mut s) => {
-                    s.raw = new_val.into();
-                }
-                None => {
-                    self.references = Some(References {
-                        raw: new_val.into(),
-                        refs: Vec::new(),
-                    });
+                    self.references = References::new(value);
                 }
             }
+            self.other_headers_mut().insert(
+                HeaderName::REFERENCES,
+                String::from_utf8_lossy(new_val).to_string(),
+            );
         }
         self
     }
 
-    pub fn references(&self) -> SmallVec<[&MessageID; 8]> {
+    pub fn references(&self) -> &[MessageID] {
         match self.references {
-            Some(ref s) => s.refs.iter().fold(SmallVec::new(), |mut acc, x| {
-                acc.push(x);
-                acc
-            }),
-            None => SmallVec::new(),
+            Some(ref s) => s.refs(),
+            None => &[],
         }
     }
 

melib/src/email/address.rs

@@ -658,15 +658,63 @@ impl Hash for MessageID {
     }
 }
 
-#[derive(Clone, Deserialize, Serialize)]
+#[derive(Clone, Deserialize, Eq, Hash, PartialEq, Serialize)]
 pub struct References {
-    pub raw: Vec<u8>,
-    pub refs: Vec<MessageID>,
+    refs: Vec<MessageID>,
 }
 
 impl std::fmt::Debug for References {
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-        write!(f, "{:#?}", self.refs)
+    fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
+        let mut dbg_t = fmt.debug_tuple(crate::identify! {References});
+        for r in &self.refs {
+            dbg_t.field(r);
+        }
+        dbg_t.finish()
+    }
+}
+
+impl References {
+    pub fn new(refs: Vec<MessageID>) -> Option<Self> {
+        if refs.is_empty() {
+            return None;
+        }
+        Some(Self { refs })
+    }
+
+    pub fn push(&mut self, new: MessageID) {
+        self.refs.push(new);
+    }
+
+    /// A parent reference should only be removed in order to break cycles (when
+    /// an envelope refers to its own `Message-ID` as a parent).
+    pub fn remove(&mut self, msgid: &MessageID) {
+        self.refs.retain(|r| r != msgid);
+    }
+
+    pub fn refs(&self) -> &[MessageID] {
+        &self.refs
+    }
+}
+
+impl Extend<MessageID> for References {
+    /// Insert new [`MessageID`] values, de-duplicated.
+    fn extend<T: IntoIterator<Item = MessageID>>(&mut self, iter: T) {
+        for elem in iter {
+            if !self.refs.contains(&elem) {
+                self.refs.push(elem);
+            }
+        }
+    }
+}
+
+impl<'a> Extend<&'a MessageID> for References {
+    /// Insert new [`MessageID`] values, de-duplicated.
+    fn extend<T: IntoIterator<Item = &'a MessageID>>(&mut self, iter: T) {
+        for elem in iter {
+            if !self.refs.contains(elem) {
+                self.refs.push(elem.clone());
+            }
+        }
     }
 }
 

melib/src/imap/protocol_parser.rs

@@ -1221,8 +1221,8 @@ pub fn envelope(input: &[u8]) -> IResult<&[u8], Envelope> {
             }
             if let Some(in_reply_to) = in_reply_to {
                 env.set_in_reply_to(&in_reply_to);
-                if let Some(in_reply_to) = env.in_reply_to().cloned() {
-                    env.push_references(in_reply_to);
+                if let Some(in_reply_to) = env.in_reply_to().map(|r| r.as_ref().clone()) {
+                    env.push_references(&in_reply_to);
                 }
             }
 

melib/src/jmap/email.rs

@@ -286,15 +286,15 @@ impl From<EmailObject> for crate::Envelope {
         if let Some(v) = t.message_id.first() {
             env.set_message_id(v.as_bytes());
         }
+        if let Some(v) = t.headers.get(HeaderName::REFERENCES.as_str()) {
+            env.set_references(v.as_bytes());
+        }
         if let Some(ref in_reply_to) = t.in_reply_to {
             env.set_in_reply_to(in_reply_to[0].as_bytes());
-            if let Some(in_reply_to) = env.in_reply_to().cloned() {
-                env.push_references(in_reply_to);
+            if let Some(in_reply_to) = env.in_reply_to().map(|r| r.as_ref().clone()) {
+                env.push_references(&in_reply_to);
             }
         }
-        if let Some(v) = t.headers.get(HeaderName::REFERENCES.as_str()) {
-            env.set_references(v.as_bytes());
-        }
         if let Some(v) = t.headers.get(HeaderName::DATE.as_str()) {
             env.set_date(v.as_bytes());
             if let Ok(d) = rfc5322_date(v.as_bytes()) {
@@ -372,7 +372,7 @@ impl From<EmailObject> for crate::Envelope {
         }
 
         if let (Some(ref mut r), message_id) = (&mut env.references, &env.message_id) {
-            r.refs.retain(|r| r != message_id);
+            r.remove(message_id);
         }
 
         env

melib/src/thread.rs

@@ -606,9 +606,9 @@ pub struct Threads {
     tree_index: Arc<RwLock<Vec<ThreadNodeHash>>>,
     pub groups: HashMap<ThreadHash, ThreadGroup>,
 
-    message_ids: HashMap<Vec<u8>, ThreadNodeHash>,
-    pub message_ids_set: HashSet<Vec<u8>>,
-    pub missing_message_ids: HashSet<Vec<u8>>,
+    message_ids: HashMap<MessageID, ThreadNodeHash>,
+    pub message_ids_set: HashSet<MessageID>,
+    pub missing_message_ids: HashSet<MessageID>,
     pub hash_set: HashSet<EnvelopeHash>,
     pub thread_to_envelope: HashMap<ThreadHash, Vec<EnvelopeHash>>,
     pub envelope_to_thread: HashMap<EnvelopeHash, ThreadHash>,
@@ -669,13 +669,13 @@ impl Threads {
         let thread_nodes: HashMap<ThreadNodeHash, ThreadNode> =
             HashMap::with_capacity_and_hasher((length as f64 * 1.2) as usize, Default::default());
         /* A hash table of Message IDs */
-        let message_ids: HashMap<Vec<u8>, ThreadNodeHash> =
+        let message_ids: HashMap<MessageID, ThreadNodeHash> =
             HashMap::with_capacity_and_hasher(length, Default::default());
         /* A hash set of Message IDs we haven't encountered yet as an Envelope */
-        let missing_message_ids: HashSet<Vec<u8>> =
+        let missing_message_ids: HashSet<MessageID> =
             HashSet::with_capacity_and_hasher(length, Default::default());
         /* A hash set of Message IDs we have encountered as a MessageID */
-        let message_ids_set: HashSet<Vec<u8>> =
+        let message_ids_set: HashSet<MessageID> =
             HashSet::with_capacity_and_hasher(length, Default::default());
         let hash_set: HashSet<EnvelopeHash> =
             HashSet::with_capacity_and_hasher(length, Default::default());
@@ -791,7 +791,7 @@ impl Threads {
             }
         }
         if let Some((message_id, _)) = self.message_ids.iter().find(|(_, h)| **h == t_id) {
-            self.missing_message_ids.insert(message_id.to_vec());
+            self.missing_message_ids.insert(message_id.clone());
         }
     }
 
@@ -901,7 +901,7 @@ impl Threads {
             if !envelopes_lck.contains_key(&env_hash) {
                 return false;
             }
-            let message_id = envelopes_lck[&env_hash].message_id().raw();
+            let message_id = envelopes_lck[&env_hash].message_id();
             if self.message_ids.contains_key(message_id)
                 && !self.missing_message_ids.contains(message_id)
             {
@@ -924,12 +924,15 @@ impl Threads {
             }
         }
         let envelopes_lck = envelopes.read().unwrap();
-        let message_id = envelopes_lck[&env_hash].message_id().raw();
-        let reply_to_id: Option<ThreadNodeHash> = envelopes_lck[&env_hash]
-            .in_reply_to()
-            .map(StrBuild::raw)
+        let message_id = envelopes_lck[&env_hash].message_id();
+        let reply_to_id: Option<ThreadNodeHash> =
+            envelopes_lck[&env_hash].in_reply_to().and_then(|r| {
+                r.refs()
+                    .iter()
+                    .rev()
                     .filter(|irt| irt != &message_id)
-            .and_then(|r| self.message_ids.get(r).cloned());
+                    .find_map(|r| self.message_ids.get(r).cloned())
+            });
 
         if other_mailbox
             && reply_to_id.is_none()
@@ -937,7 +940,7 @@ impl Threads {
             && !envelopes_lck[&env_hash]
                 .references()
                 .iter()
-                .any(|r| self.message_ids.contains_key(r.raw()))
+                .any(|r| self.message_ids.contains_key(r))
         {
             return false;
         }
@@ -953,7 +956,7 @@ impl Threads {
                     None
                 },
             )
-            .unwrap_or_else(|| ThreadNodeHash::from(message_id));
+            .unwrap_or_else(|| ThreadNodeHash::from(message_id.raw()));
         {
             let node = self.thread_nodes.entry(new_id).or_default();
             node.message = Some(env_hash);
@@ -999,8 +1002,8 @@ impl Threads {
             };
         }
 
-        self.message_ids.insert(message_id.to_vec(), new_id);
-        self.message_ids_set.insert(message_id.to_vec());
+        self.message_ids.insert(message_id.clone(), new_id);
+        self.message_ids_set.insert(message_id.clone());
         self.missing_message_ids.remove(message_id);
         self.hash_set.insert(env_hash);
         self.thread_to_envelope
@@ -1012,10 +1015,20 @@ impl Threads {
             make!((reply_to_id) parent of (new_id), self);
         } else if let Some(r) = envelopes_lck[&env_hash]
             .in_reply_to()
-            .map(StrBuild::raw)
-            .filter(|irt| irt != &message_id)
+            .clone()
+            .into_iter()
+            .find_map(|r| {
+                r.refs()
+                    .iter()
+                    .rev()
+                    .filter(|irt| *irt != message_id)
+                    .find(|r| {
+                        self.message_ids.contains_key(r) && !self.missing_message_ids.contains(r)
+                    })
+                    .cloned()
+            })
         {
-            let reply_to_id = ThreadNodeHash::from(r);
+            let reply_to_id = ThreadNodeHash::from(&r.raw());
             self.thread_nodes.insert(
                 reply_to_id,
                 ThreadNode {
@@ -1036,25 +1049,30 @@ impl Threads {
                 }),
             );
             make!((reply_to_id) parent of (new_id), self);
-            self.message_ids.insert(r.to_vec(), reply_to_id);
-            self.message_ids_set.insert(r.to_vec());
-            self.missing_message_ids.insert(r.to_vec());
+            self.message_ids.insert(r.clone(), reply_to_id);
+            self.message_ids_set.insert(r.clone());
+            self.missing_message_ids.insert(r);
         }
 
         if envelopes_lck[&env_hash].references.is_some() {
             let mut current_descendant_id = new_id;
-            let mut references = envelopes_lck[&env_hash].references();
-            if references.first().filter(|irt| irt.raw() != message_id)
-                == envelopes_lck[&env_hash].in_reply_to().as_ref()
+            let mut references = envelopes_lck[&env_hash].references().to_vec();
+            if envelopes_lck[&env_hash]
+                .in_reply_to()
+                .map(|r| {
+                    r.refs().last().as_ref()
+                        == references.first().filter(|irt| *irt != message_id).as_ref()
+                })
+                .unwrap_or(false)
             {
                 references.reverse();
             }
 
             for reference in references.into_iter().rev() {
-                if reference.raw() == message_id {
+                if &reference == message_id {
                     continue;
                 }
-                if let Some(&id) = self.message_ids.get(reference.raw()) {
+                if let Some(&id) = self.message_ids.get(&reference) {
                     if self.thread_nodes[&id].date > self.thread_nodes[&current_descendant_id].date
                         || self.thread_nodes[&current_descendant_id].parent.is_some()
                     {
@@ -1084,9 +1102,9 @@ impl Threads {
                         }),
                     );
                     make!((id) parent of (current_descendant_id), self);
-                    self.missing_message_ids.insert(reference.raw().to_vec());
-                    self.message_ids.insert(reference.raw().to_vec(), id);
-                    self.message_ids_set.insert(reference.raw().to_vec());
+                    self.missing_message_ids.insert(reference.clone());
+                    self.message_ids.insert(reference.clone(), id);
+                    self.message_ids_set.insert(reference.clone());
                     current_descendant_id = id;
                 }
             }
@@ -1108,7 +1126,7 @@ impl Threads {
                 .message_ids
                 .iter()
                 .map(|(a, &b)| (b, a.to_vec()))
-                .collect::<HashMap<ThreadNodeHash, Vec<u8>>>(),
+                .collect::<HashMap<ThreadNodeHash, MessageID>>(),
             &envelopes,
         );
         */
@@ -1507,7 +1525,7 @@ fn print_threadnodes(
 fn save_graph(
     node_arr: &[ThreadNodeHash],
     nodes: &HashMap<ThreadNodeHash, ThreadNode>,
-    ids: &HashMap<ThreadNodeHash, Vec<u8>>,
+    ids: &HashMap<ThreadNodeHash, MessageID>,
     envelopes: &Envelopes,
 ) {
     let envelopes = envelopes.read().unwrap();