fix: parsing multi-properties

the parser now differentiates between different kinds of properties that may occur multiple times in a component and will no longer override them with each other.
hoodie · Nov 22, 2023 · c0ffee9 · c0ffee9
1 parent 7ed752d
commit c0ffee9
Show file tree

Hide file tree

Showing 4 changed files with 121 additions and 10 deletions.
diff --git a/src/components.rs b/src/components.rs
@@ -23,7 +23,7 @@ pub use venue::*;
 #[derive(Debug, Default, PartialEq, Eq, Clone)]
 pub(crate) struct InnerComponent {
     pub properties: BTreeMap<String, Property>,
-    pub multi_properties: Vec<Property>,
+    pub multi_properties: BTreeMap<String, Vec<Property>>,
     pub components: Vec<Other>,
 }
 
@@ -50,6 +50,17 @@ impl InnerComponent {
         }
     }
 
+    pub(crate) fn insert_multi(&mut self, property: impl Into<Property>) -> &mut Self {
+        let property = property.into();
+        let key = property.key().to_owned();
+
+        self.multi_properties
+            .entry(key)
+            .and_modify(|v| v.push(property.to_owned()))
+            .or_insert(vec![property.to_owned()]);
+        self
+    }
+
     #[cfg(test)]
     pub fn property_value(&self, key: &str) -> Option<&str> {
         Some(self.properties.get(key)?.value())
@@ -72,7 +83,7 @@ pub trait Component {
     fn components(&self) -> &[Other];
 
     /// Read-only access to `multi_properties`
-    fn multi_properties(&self) -> &Vec<Property>;
+    fn multi_properties(&self) -> &BTreeMap<String, Vec<Property>>;
 
     /// Gets the value of a property.
     fn property_value(&self, key: &str) -> Option<&str> {
@@ -96,7 +107,7 @@ pub trait Component {
             write_crlf!(out, "UID:{}", Uuid::new_v4())?;
         }
 
-        for property in self.multi_properties() {
+        for property in self.multi_properties().values().flatten() {
             property.fmt_write(out)?;
         }
 
@@ -355,7 +366,7 @@ macro_rules! component_impl {
             }
 
             /// Read-only access to `multi_properties`
-            fn multi_properties(&self) -> &Vec<Property> {
+            fn multi_properties(&self) -> &BTreeMap<String, Vec<Property>> {
                 &self.inner.multi_properties
             }
 
@@ -375,7 +386,7 @@ macro_rules! component_impl {
 
             /// Adds a [`Property`] of which there may be many
             fn append_multi_property(&mut self, property: impl Into<Property>) -> &mut Self {
-                self.inner.multi_properties.push(property.into());
+                self.inner.insert_multi(property);
                 self
             }
         }

diff --git a/src/components/other.rs b/src/components/other.rs
@@ -25,7 +25,7 @@ impl Component for Other {
     }
 
     /// Read-only access to `multi_properties`
-    fn multi_properties(&self) -> &Vec<Property> {
+    fn multi_properties(&self) -> &BTreeMap<String, Vec<Property>> {
         &self.inner.multi_properties
     }
 
@@ -40,7 +40,7 @@ impl Component for Other {
 
     /// Adds a `Property` of which there may be many
     fn append_multi_property(&mut self, property: impl Into<Property>) -> &mut Self {
-        self.inner.multi_properties.push(property.into());
+        self.inner.insert_multi(property);
         self
     }
 

diff --git a/src/parser/components.rs b/src/parser/components.rs
@@ -131,15 +131,26 @@ impl From<Component<'_>> for Other {
 
 impl From<Component<'_>> for InnerComponent {
     fn from(component: Component) -> Self {
-        Self {
+        let mut from_component = Self {
             properties: component
                 .properties
-                .into_iter()
-                .map(|p| (p.name.clone().into_owned().into(), p.into()))
+                .iter()
+                .filter(|p| !p.is_multi_property())
+                .map(|p| (p.name.clone().into_owned().into(), p.to_owned().into()))
                 .collect(),
             components: component.components.into_iter().map(Other::from).collect(),
             multi_properties: Default::default(),
+        };
+
+        for p in component
+            .properties
+            .into_iter()
+            .filter(Property::is_multi_property)
+        {
+            from_component.insert_multi(p);
         }
+
+        from_component
     }
 }
 
@@ -271,6 +282,18 @@ pub fn component<'a, E: ParseError<&'a str> + ContextError<&'a str>>(
     ))
 }
 
+#[cfg(test)]
+pub fn inner_component<'a, E: ParseError<&'a str> + ContextError<&'a str>>(
+    input: &'a str,
+) -> IResult<&'a str, InnerComponent, E> {
+    match component::<(_, _)>(input) {
+        Ok(result) => {
+            return Ok((result.0, InnerComponent::from(result.1)));
+        }
+        Err(_e) => todo!(),
+    };
+}
+
 #[test]
 fn test_components() {
     assert_parser!(component, "BEGIN:FOO\nEND:FOO", Component::new_empty("FOO"));
@@ -424,6 +447,60 @@ END:VEVENT
     );
 }
 
+#[test]
+fn test_multi_properties() {
+    let multi_properties = From::from([(
+        "ATTENDEE".into(),
+        vec![
+            Property {
+                name: "ATTENDEE".into(),
+                val: "mailto:email@example.com".into(),
+                params: vec![
+                    Parameter {
+                        key: "EMAIL".into(),
+                        val: Some("\"email@example.com\"".into()),
+                    },
+                    Parameter {
+                        key: "CUTYPE".into(),
+                        val: Some("INDIVIDUAL".into()),
+                    },
+                ],
+            }
+            .into(),
+            Property {
+                name: "ATTENDEE".into(),
+                val: "mailto:dmail@example.com".into(),
+                params: vec![
+                    Parameter {
+                        key: "EMAIL".into(),
+                        val: Some("\"dmail@example.com\"".into()),
+                    },
+                    Parameter {
+                        key: "CUTYPE".into(),
+                        val: Some("INDIVIDUAL".into()),
+                    },
+                ],
+            }
+            .into(),
+        ],
+    )]);
+
+    assert_parser!(
+        inner_component,
+        r#"
+BEGIN:VEVENT
+ATTENDEE;CUTYPE=INDIVIDUAL;EMAIL="email@example.com":mailto:email@example.com
+ATTENDEE;CUTYPE=INDIVIDUAL;EMAIL="dmail@example.com":mailto:dmail@example.com
+END:VEVENT
+"#,
+        InnerComponent {
+            properties: Default::default(),
+            multi_properties,
+            components: vec![]
+        }
+    );
+}
+
 #[test]
 fn test_faulty_component() {
     use nom::error::{ErrorKind::*, VerboseErrorKind::*};

diff --git a/src/parser/properties.rs b/src/parser/properties.rs
@@ -23,6 +23,25 @@ use nom::{
 #[cfg(test)]
 use nom::error::ErrorKind;
 
+/// [RFC-5545](https://datatracker.ietf.org/doc/html/rfc5545) states that the following
+/// "MAY occur more than once" in a VEVENT, VTODO, VJOURNAL, and VFREEBUSY.
+/// Note: A VJOURNAL can also contain multiple DECRIPTION but this is not covered here.
+const MULTIS: [&str; 13] = [
+    "ATTACH",
+    "ATTENDEE",
+    "CATEGORIES",
+    "COMMENT",
+    "CONTACT",
+    "EXDATE",
+    "FREEBUSY",
+    "IANA-PROP",
+    "RDATE",
+    "RELATED",
+    "RESOURCES",
+    "RSTATUS",
+    "X-PROP",
+];
+
 /// Zero-copy version of [`crate::properties::Property`]
 #[derive(PartialEq, Eq, Debug, Clone)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
@@ -59,6 +78,10 @@ impl Property<'_> {
         write_crlf!(out, "{}", fold_line(&line))?;
         Ok(())
     }
+
+    pub(crate) fn is_multi_property(&self) -> bool {
+        MULTIS.contains(&self.name.as_str())
+    }
 }
 
 impl fmt::Display for Property<'_> {