From 550b6499f8e9347b9e0026ad9cdb5096606047b7 Mon Sep 17 00:00:00 2001
From: Dave Terra <11788035+daveterra@users.noreply.github.com>
Date: Thu, 16 Nov 2023 13:24:59 -0800
Subject: [PATCH] Addressing some PR comments

- Moved inner function for multi-check into property impl. This also now checks for additional properties. The only remaining thing here is the special case for DESCRIPTION on VJOURNAL
- Iterating over properties instead of splitting into two vec
---
 src/components.rs        |  6 ++----
 src/parser/components.rs | 22 +++++++++++-----------
 src/parser/properties.rs | 22 ++++++++++++++++++++++
 3 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/src/components.rs b/src/components.rs
index e9b87f94..84ec7f8e 100644
--- a/src/components.rs
+++ b/src/components.rs
@@ -107,10 +107,8 @@ pub trait Component {
             write_crlf!(out, "UID:{}", Uuid::new_v4())?;
         }
 
-        for properties in self.multi_properties().values() {
-            for property in properties {
-                property.fmt_write(out)?;
-            }
+        for property in self.multi_properties().values().flatten() {
+            property.fmt_write(out)?;
         }
 
         for component in self.components() {
diff --git a/src/parser/components.rs b/src/parser/components.rs
index e0489253..bd2295d6 100644
--- a/src/parser/components.rs
+++ b/src/parser/components.rs
@@ -131,23 +131,22 @@ impl From<Component<'_>> for Other {
 
 impl From<Component<'_>> for InnerComponent {
     fn from(component: Component) -> Self {
-        fn is_multi(property: &Property) -> bool {
-            property.name.to_string() == "ATTENDEE"
-        }
-
-        let (multi, single): (Vec<_>, Vec<_>) =
-            component.properties.into_iter().partition(is_multi);
-
         let mut from_component = Self {
-            properties: single
-                .into_iter()
-                .map(|p| (p.name.clone().into_owned().into(), p.into()))
+            properties: component
+                .properties
+                .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 multi.into_iter() {
+        for p in component
+            .properties
+            .into_iter()
+            .filter(|p| p.is_multi_property())
+        {
             from_component.insert_multi(p);
         }
 
@@ -487,6 +486,7 @@ fn test_multi_properties() {
             .into(),
         ],
     );
+
     assert_parser!(
         inner_component,
         r#"
diff --git a/src/parser/properties.rs b/src/parser/properties.rs
index b0f0005e..839bbcc6 100644
--- a/src/parser/properties.rs
+++ b/src/parser/properties.rs
@@ -59,6 +59,28 @@ impl Property<'_> {
         write_crlf!(out, "{}", fold_line(&line))?;
         Ok(())
     }
+
+    pub(crate) fn is_multi_property(&self) -> bool {
+        // [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.
+        [
+            "ATTACH",
+            "ATTENDEE",
+            "CATEGORIES",
+            "COMMENT",
+            "CONTACT",
+            "EXDATE",
+            "FREEBUSY",
+            "IANA-PROP",
+            "RDATE",
+            "RELATED",
+            "RESOURCES",
+            "RSTATUS",
+            "X-PROP",
+        ]
+        .contains(&self.name.as_str())
+    }
 }
 
 impl fmt::Display for Property<'_> {