Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: support for multi-properties #82

Merged
merged 1 commit into from
Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions src/components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't you want to go with a multimap here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still can, it does add another dependencies and I thought this approach might be more simple. It also keeps multi_properties more similar to properties in type. Happy to change it to multimap if you prefer.

pub components: Vec<Other>,
}

Expand All @@ -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())
Expand All @@ -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> {
Expand All @@ -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)?;
}

Expand Down Expand Up @@ -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
}

Expand All @@ -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
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/components/other.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down
83 changes: 80 additions & 3 deletions src/parser/components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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"));
Expand Down Expand Up @@ -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 protected]".into(),
params: vec![
Parameter {
key: "EMAIL".into(),
val: Some("\"[email protected]\"".into()),
},
Parameter {
key: "CUTYPE".into(),
val: Some("INDIVIDUAL".into()),
},
],
}
.into(),
Property {
name: "ATTENDEE".into(),
val: "mailto:[email protected]".into(),
params: vec![
Parameter {
key: "EMAIL".into(),
val: Some("\"[email protected]\"".into()),
},
Parameter {
key: "CUTYPE".into(),
val: Some("INDIVIDUAL".into()),
},
],
}
.into(),
],
)]);

assert_parser!(
inner_component,
r#"
BEGIN:VEVENT
ATTENDEE;CUTYPE=INDIVIDUAL;EMAIL="[email protected]":mailto:[email protected]
ATTENDEE;CUTYPE=INDIVIDUAL;EMAIL="[email protected]":mailto:[email protected]
END:VEVENT
"#,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for simplicity you could also just test parsing and reserializing this same event

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, interesting Idea. I will try this.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm, don't have to, I tried, it's a bit of a waste of time ;)

InnerComponent {
properties: Default::default(),
multi_properties,
components: vec![]
}
);
}

#[test]
fn test_faulty_component() {
use nom::error::{ErrorKind::*, VerboseErrorKind::*};
Expand Down
23 changes: 23 additions & 0 deletions src/parser/properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down Expand Up @@ -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<'_> {
Expand Down
Loading