feat: add quirk to dedup and sort sequence sets during encoding

Author: soywod

imap-codec/Cargo.toml

@@ -35,6 +35,7 @@ quirk = [
     "quirk_spaces_between_addresses",
     "quirk_empty_continue_req",
     "quirk_body_fld_enc_nil_to_empty",
+    "quirk_dedup_sort_seq_encoding",
 ]
 # Make `\r` in `\r\n` optional.
 quirk_crlf_relaxed = []
@@ -60,6 +61,8 @@ quirk_trailing_space_search = []
 quirk_empty_continue_req = []
 # Encode NIL `body-fld-enc` as empty string.
 quirk_body_fld_enc_nil_to_empty = []
+# Dedup and sort sequence sets during encoding
+quirk_dedup_sort_seq_encoding = []
 
 # <Forward to imap-types>
 arbitrary = ["imap-types/arbitrary"]

imap-codec/src/codec/encode.rs

@@ -1024,16 +1024,61 @@ impl EncodeIntoContext for SearchKey<'_> {
 }
 
 impl EncodeIntoContext for SequenceSet {
+    #[cfg(not(feature = "quirk_dedup_sort_seq_encoding"))]
     fn encode_ctx(&self, ctx: &mut EncodeContext) -> std::io::Result<()> {
         join_serializable(self.0.as_ref(), b",", ctx)
     }
+
+    #[cfg(feature = "quirk_dedup_sort_seq_encoding")]
+    fn encode_ctx(&self, ctx: &mut EncodeContext) -> std::io::Result<()> {
+        let mut seq_set = self.0.as_ref().to_vec();
+
+        if seq_set.len() == 1 {
+            return seq_set.remove(0).encode_ctx(ctx);
+        }
+
+        seq_set.sort_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal));
+
+        let mut a = 0;
+        let mut b = 1;
+
+        while b < seq_set.len() {
+            if seq_set[a].partial_cmp(&seq_set[b]).is_some() {
+                a += 1;
+                b += 1;
+                continue;
+            }
+
+            match (&seq_set[a], &seq_set[b]) {
+                (Sequence::Single(_), _) => {
+                    seq_set.remove(a);
+                }
+                (Sequence::Range(_, _), Sequence::Single(_)) => {
+                    seq_set.remove(b);
+                }
+                (Sequence::Range(a1, a2), Sequence::Range(b1, b2)) => {
+                    let min = a1.clone().min(a2.clone()).min(b1.clone()).min(b2.clone());
+                    let max = a1.clone().max(a2.clone()).max(b1.clone()).max(b2.clone());
+                    let _ = std::mem::replace(&mut seq_set[a], Sequence::Range(min, max));
+                    seq_set.remove(b);
+                }
+            }
+        }
+
+        join_serializable(&seq_set, b",", ctx)
+    }
 }
 
 impl EncodeIntoContext for Sequence {
     fn encode_ctx(&self, ctx: &mut EncodeContext) -> std::io::Result<()> {
         match self {
             Sequence::Single(seq_no) => seq_no.encode_ctx(ctx),
             Sequence::Range(from, to) => {
+                #[cfg(feature = "quirk_dedup_sort_seq_encoding")]
+                let from = from.min(to);
+                #[cfg(feature = "quirk_dedup_sort_seq_encoding")]
+                let to = from.max(to);
+
                 from.encode_ctx(ctx)?;
                 ctx.write_all(b":")?;
                 to.encode_ctx(ctx)
@@ -2128,6 +2173,32 @@ mod tests {
 
     use super::*;
 
+    #[cfg(feature = "quirk_dedup_sort_seq_encoding")]
+    #[test]
+    fn test_sequence_reordering() {
+        let tests = [
+            ("1,2,3,4", "1,2,3,4"),
+            ("3,1,2,4", "1,2,3,4"),
+            ("3:1,5", "1:3,5"),
+            ("5,3:1", "1:3,5"),
+            ("3,3:1", "1:3"),
+            ("3:1,1", "1:3"),
+            ("3:1,2:5", "1:5"),
+            ("3:1,4:9", "1:3,4:9"),
+            ("9:4,3:1", "1:3,4:9"),
+            ("8:10,3:1,2:5,9", "1:5,8:10"),
+        ];
+
+        for (expected, got) in tests {
+            let mut ctx = EncodeContext::default();
+            SequenceSet::try_from(expected)
+                .unwrap()
+                .encode_ctx(&mut ctx)
+                .unwrap();
+            assert_eq!(got, escape_byte_string(&ctx.accumulator));
+        }
+    }
+
     #[test]
     fn test_api_encoder_usage() {
         let cmd = Command::new(

imap-types/src/sequence.rs

@@ -1,5 +1,5 @@
 use std::{
-    cmp::max,
+    cmp::{Ordering, max},
     collections::VecDeque,
     fmt::Debug,
     iter::Rev,
@@ -140,6 +140,47 @@ pub enum Sequence {
     Range(SeqOrUid, SeqOrUid),
 }
 
+impl PartialOrd for Sequence {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        match (self, other) {
+            (Sequence::Single(a), Sequence::Single(b)) => a.partial_cmp(b),
+            (Sequence::Single(a), Sequence::Range(b1, b2)) => {
+                if a < b1.min(b2) {
+                    return Some(Ordering::Less);
+                }
+
+                if a > b1.max(b2) {
+                    return Some(Ordering::Greater);
+                }
+
+                None
+            }
+            (Sequence::Range(a1, a2), Sequence::Single(b)) => {
+                if b < a1.min(a2) {
+                    return Some(Ordering::Greater);
+                }
+
+                if b > a1.max(a2) {
+                    return Some(Ordering::Less);
+                }
+
+                None
+            }
+            (Sequence::Range(a1, a2), Sequence::Range(b1, b2)) => {
+                if a1.max(a2) < b1 && a1.max(a2) < b2 {
+                    return Some(Ordering::Less);
+                }
+
+                if a1.min(a2) > b1 && a1.min(a2) > b2 {
+                    return Some(Ordering::Greater);
+                }
+
+                None
+            }
+        }
+    }
+}
+
 impl From<SeqOrUid> for Sequence {
     fn from(value: SeqOrUid) -> Self {
         Self::Single(value)
@@ -192,6 +233,23 @@ pub enum SeqOrUid {
     Asterisk,
 }
 
+impl Ord for SeqOrUid {
+    fn cmp(&self, other: &Self) -> Ordering {
+        match (self, other) {
+            (SeqOrUid::Asterisk, SeqOrUid::Asterisk) => Ordering::Equal,
+            (_, SeqOrUid::Asterisk) => Ordering::Greater,
+            (SeqOrUid::Asterisk, _) => Ordering::Less,
+            (SeqOrUid::Value(a), SeqOrUid::Value(b)) => a.cmp(b),
+        }
+    }
+}
+
+impl PartialOrd for SeqOrUid {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
 impl From<NonZeroU32> for SeqOrUid {
     fn from(value: NonZeroU32) -> Self {
         Self::Value(value)