Some refactoring work

crates/caldav/src/calendar/methods/report/calendar_multiget.rs

@@ -10,7 +10,7 @@ use actix_web::{
 };
 use rustical_dav::{
     resource::Resource,
-    xml::{MultistatusElement, PropElement, PropfindType, multistatus::ResponseElement},
+    xml::{MultistatusElement, PropfindType, multistatus::ResponseElement},
 };
 use rustical_store::{CalendarObject, CalendarStore, auth::User};
 use rustical_xml::XmlDeserialize;
@@ -56,7 +56,8 @@ pub async fn get_objects_calendar_multiget<C: CalendarStore>(
 }
 
 pub async fn handle_calendar_multiget<C: CalendarStore>(
-    cal_multiget: CalendarMultigetRequest,
+    cal_multiget: &CalendarMultigetRequest,
+    props: &[&str],
     req: HttpRequest,
     user: &User,
     principal: &str,
@@ -64,26 +65,9 @@ pub async fn handle_calendar_multiget<C: CalendarStore>(
     cal_store: &C,
 ) -> Result<MultistatusElement<CalendarObjectPropWrapper, String>, Error> {
     let (objects, not_found) =
-        get_objects_calendar_multiget(&cal_multiget, req.path(), principal, cal_id, cal_store)
+        get_objects_calendar_multiget(cal_multiget, req.path(), principal, cal_id, cal_store)
             .await?;
 
-    let props = match cal_multiget.prop {
-        PropfindType::Allprop => {
-            vec!["allprop".to_owned()]
-        }
-        PropfindType::Propname => {
-            vec!["propname".to_owned()]
-        }
-        PropfindType::Prop(PropElement(prop_tags)) => prop_tags
-            .into_iter()
-            .map(|propname| match propname {
-                ReportPropName::Propname(propname) => propname.0,
-                ReportPropName::CalendarData(_) => "calendar-data".to_owned(),
-            })
-            .collect(),
-    };
-    let props: Vec<&str> = props.iter().map(String::as_str).collect();
-
     let mut responses = Vec::new();
     for object in objects {
         let path = format!("{}/{}.ics", req.path(), object.get_id());
@@ -92,7 +76,7 @@ pub async fn handle_calendar_multiget<C: CalendarStore>(
                 object,
                 principal: principal.to_owned(),
             }
-            .propfind(&path, &props, user, req.resource_map())?,
+            .propfind(&path, props, user, req.resource_map())?,
         );
     }
 

crates/caldav/src/calendar/methods/report/calendar_query.rs

@@ -1,7 +1,7 @@
 use actix_web::HttpRequest;
 use rustical_dav::{
     resource::Resource,
-    xml::{MultistatusElement, PropElement, PropfindType},
+    xml::{MultistatusElement, PropfindType},
 };
 use rustical_store::{
     CalendarObject, CalendarStore, auth::User, calendar::UtcDateTime, calendar_store::CalendarQuery,
@@ -216,31 +216,15 @@ pub async fn get_objects_calendar_query<C: CalendarStore>(
 }
 
 pub async fn handle_calendar_query<C: CalendarStore>(
-    cal_query: CalendarQueryRequest,
+    cal_query: &CalendarQueryRequest,
+    props: &[&str],
     req: HttpRequest,
     user: &User,
     principal: &str,
     cal_id: &str,
     cal_store: &C,
 ) -> Result<MultistatusElement<CalendarObjectPropWrapper, String>, Error> {
-    let objects = get_objects_calendar_query(&cal_query, principal, cal_id, cal_store).await?;
+    let objects = get_objects_calendar_query(cal_query, principal, cal_id, cal_store).await?;
-
-    let props = match cal_query.prop {
-        PropfindType::Allprop => {
-            vec!["allprop".to_owned()]
-        }
-        PropfindType::Propname => {
-            vec!["propname".to_owned()]
-        }
-        PropfindType::Prop(PropElement(prop_tags)) => prop_tags
-            .into_iter()
-            .map(|propname| match propname {
-                ReportPropName::Propname(propname) => propname.0,
-                ReportPropName::CalendarData(_) => "calendar-data".to_owned(),
-            })
-            .collect(),
-    };
-    let props: Vec<&str> = props.iter().map(String::as_str).collect();
 
     let mut responses = Vec::new();
     for object in objects {
@@ -254,7 +238,7 @@ pub async fn handle_calendar_query<C: CalendarStore>(
                 object,
                 principal: principal.to_owned(),
             }
-            .propfind(&path, &props, user, req.resource_map())?,
+            .propfind(&path, props, user, req.resource_map())?,
         );
     }
 

crates/caldav/src/calendar/methods/report/mod.rs

@@ -5,7 +5,9 @@ use actix_web::{
 };
 use calendar_multiget::{CalendarMultigetRequest, handle_calendar_multiget};
 use calendar_query::{CalendarQueryRequest, handle_calendar_query};
-use rustical_dav::xml::{Propname, sync_collection::SyncCollectionRequest};
+use rustical_dav::xml::{
+    PropElement, PropfindType, Propname, sync_collection::SyncCollectionRequest,
+};
 use rustical_store::{CalendarStore, auth::User};
 use rustical_xml::{XmlDeserialize, XmlDocument};
 use sync_collection::handle_sync_collection;
@@ -53,6 +55,34 @@ pub(crate) enum ReportRequest {
     SyncCollection(SyncCollectionRequest<ReportPropName>),
 }
 
+impl ReportRequest {
+    fn props(&self) -> Vec<&str> {
+        let prop_element = match self {
+            ReportRequest::CalendarMultiget(CalendarMultigetRequest { prop, .. }) => prop,
+            ReportRequest::CalendarQuery(CalendarQueryRequest { prop, .. }) => prop,
+            ReportRequest::SyncCollection(SyncCollectionRequest { prop, .. }) => prop,
+        };
+
+        let props = match prop_element {
+            PropfindType::Allprop => {
+                vec!["allprop"]
+            }
+            PropfindType::Propname => {
+                vec!["propname"]
+            }
+            PropfindType::Prop(PropElement(prop_tags)) => prop_tags
+                .iter()
+                .map(|propname| match propname {
+                    ReportPropName::Propname(propname) => propname.0.as_str(),
+                    ReportPropName::CalendarData(_) => "calendar-data",
+                })
+                .collect(),
+        };
+        // let props: Vec<&str> = props.iter().map(String::as_str).collect();
+        props
+    }
+}
+
 #[instrument(skip(req, cal_store))]
 pub async fn route_report_calendar<C: CalendarStore>(
     path: Path<(String, String)>,
@@ -67,11 +97,13 @@ pub async fn route_report_calendar<C: CalendarStore>(
     }
 
     let request = ReportRequest::parse_str(&body)?;
+    let props = request.props();
 
-    Ok(match request.clone() {
+    Ok(match &request {
         ReportRequest::CalendarQuery(cal_query) => {
             handle_calendar_query(
                 cal_query,
+                &props,
                 req,
                 &user,
                 &principal,
@@ -83,6 +115,7 @@ pub async fn route_report_calendar<C: CalendarStore>(
         ReportRequest::CalendarMultiget(cal_multiget) => {
             handle_calendar_multiget(
                 cal_multiget,
+                &props,
                 req,
                 &user,
                 &principal,
@@ -94,6 +127,7 @@ pub async fn route_report_calendar<C: CalendarStore>(
         ReportRequest::SyncCollection(sync_collection) => {
             handle_sync_collection(
                 sync_collection,
+                &props,
                 req,
                 &user,
                 &principal,

crates/caldav/src/calendar/methods/report/sync_collection.rs

@@ -7,8 +7,7 @@ use actix_web::{HttpRequest, http::StatusCode};
 use rustical_dav::{
     resource::Resource,
     xml::{
-        MultistatusElement, PropElement, PropfindType, multistatus::ResponseElement,
+        MultistatusElement, multistatus::ResponseElement, sync_collection::SyncCollectionRequest,
-        sync_collection::SyncCollectionRequest,
     },
 };
 use rustical_store::{
@@ -18,30 +17,14 @@ use rustical_store::{
 };
 
 pub async fn handle_sync_collection<C: CalendarStore>(
-    sync_collection: SyncCollectionRequest<ReportPropName>,
+    sync_collection: &SyncCollectionRequest<ReportPropName>,
+    props: &[&str],
     req: HttpRequest,
     user: &User,
     principal: &str,
     cal_id: &str,
     cal_store: &C,
 ) -> Result<MultistatusElement<CalendarObjectPropWrapper, String>, Error> {
-    let props = match sync_collection.prop {
-        PropfindType::Allprop => {
-            vec!["allprop".to_owned()]
-        }
-        PropfindType::Propname => {
-            vec!["propname".to_owned()]
-        }
-        PropfindType::Prop(PropElement(prop_tags)) => prop_tags
-            .into_iter()
-            .map(|propname| match propname {
-                ReportPropName::Propname(propname) => propname.0,
-                ReportPropName::CalendarData(_) => "calendar-data".to_owned(),
-            })
-            .collect(),
-    };
-    let props: Vec<&str> = props.iter().map(String::as_str).collect();
-
     let old_synctoken = parse_synctoken(&sync_collection.sync_token).unwrap_or(0);
     let (new_objects, deleted_objects, new_synctoken) = cal_store
         .sync_changes(principal, cal_id, old_synctoken)
@@ -59,7 +42,7 @@ pub async fn handle_sync_collection<C: CalendarStore>(
                 object,
                 principal: principal.to_owned(),
             }
-            .propfind(&path, &props, user, req.resource_map())?,
+            .propfind(&path, props, user, req.resource_map())?,
         );
     }
 

crates/carddav/src/addressbook/methods/report/addressbook_multiget.rs

@@ -9,7 +9,7 @@ use actix_web::{
 };
 use rustical_dav::{
     resource::Resource,
-    xml::{MultistatusElement, PropElement, PropfindType, multistatus::ResponseElement},
+    xml::{MultistatusElement, PropfindType, multistatus::ResponseElement},
 };
 use rustical_store::{AddressObject, AddressbookStore, auth::User};
 use rustical_xml::XmlDeserialize;
@@ -58,7 +58,8 @@ pub async fn get_objects_addressbook_multiget<AS: AddressbookStore>(
 }
 
 pub async fn handle_addressbook_multiget<AS: AddressbookStore>(
-    addr_multiget: AddressbookMultigetRequest,
+    addr_multiget: &AddressbookMultigetRequest,
+    props: &[&str],
     req: HttpRequest,
     user: &User,
     principal: &str,
@@ -66,22 +67,9 @@ pub async fn handle_addressbook_multiget<AS: AddressbookStore>(
     addr_store: &AS,
 ) -> Result<MultistatusElement<AddressObjectPropWrapper, String>, Error> {
     let (objects, not_found) =
-        get_objects_addressbook_multiget(&addr_multiget, req.path(), principal, cal_id, addr_store)
+        get_objects_addressbook_multiget(addr_multiget, req.path(), principal, cal_id, addr_store)
             .await?;
 
-    let props = match addr_multiget.prop {
-        PropfindType::Allprop => {
-            vec!["allprop".to_owned()]
-        }
-        PropfindType::Propname => {
-            vec!["propname".to_owned()]
-        }
-        PropfindType::Prop(PropElement(prop_tags)) => {
-            prop_tags.into_iter().map(|propname| propname.0).collect()
-        }
-    };
-    let props: Vec<&str> = props.iter().map(String::as_str).collect();
-
     let mut responses = Vec::new();
     for object in objects {
         let path = format!("{}/{}.vcf", req.path(), object.get_id());
@@ -90,7 +78,7 @@ pub async fn handle_addressbook_multiget<AS: AddressbookStore>(
                 object,
                 principal: principal.to_owned(),
             }
-            .propfind(&path, &props, user, req.resource_map())?,
+            .propfind(&path, props, user, req.resource_map())?,
         );
     }
 

crates/carddav/src/addressbook/methods/report/mod.rs

@@ -1,11 +1,11 @@
 use crate::Error;
 use actix_web::{
-    web::{Data, Path},
     HttpRequest, Responder,
+    web::{Data, Path},
 };
-use addressbook_multiget::{handle_addressbook_multiget, AddressbookMultigetRequest};
+use addressbook_multiget::{AddressbookMultigetRequest, handle_addressbook_multiget};
-use rustical_dav::xml::sync_collection::SyncCollectionRequest;
+use rustical_dav::xml::{PropElement, PropfindType, sync_collection::SyncCollectionRequest};
-use rustical_store::{auth::User, AddressbookStore};
+use rustical_store::{AddressbookStore, auth::User};
 use rustical_xml::{XmlDeserialize, XmlDocument};
 use sync_collection::handle_sync_collection;
 use tracing::instrument;
@@ -21,6 +21,28 @@ pub(crate) enum ReportRequest {
     SyncCollection(SyncCollectionRequest),
 }
 
+impl ReportRequest {
+    fn props(&self) -> Vec<&str> {
+        let prop_element = match self {
+            ReportRequest::AddressbookMultiget(AddressbookMultigetRequest { prop, .. }) => prop,
+            ReportRequest::SyncCollection(SyncCollectionRequest { prop, .. }) => prop,
+        };
+
+        match prop_element {
+            PropfindType::Allprop => {
+                vec!["allprop"]
+            }
+            PropfindType::Propname => {
+                vec!["propname"]
+            }
+            PropfindType::Prop(PropElement(prop_tags)) => prop_tags
+                .iter()
+                .map(|propname| propname.0.as_str())
+                .collect(),
+        }
+    }
+}
+
 #[instrument(skip(req, addr_store))]
 pub async fn route_report_addressbook<AS: AddressbookStore>(
     path: Path<(String, String)>,
@@ -35,11 +57,13 @@ pub async fn route_report_addressbook<AS: AddressbookStore>(
     }
 
     let request = ReportRequest::parse_str(&body)?;
+    let props = request.props();
 
-    Ok(match request.clone() {
+    Ok(match &request {
         ReportRequest::AddressbookMultiget(addr_multiget) => {
             handle_addressbook_multiget(
                 addr_multiget,
+                &props,
                 req,
                 &user,
                 &principal,
@@ -51,6 +75,7 @@ pub async fn route_report_addressbook<AS: AddressbookStore>(
         ReportRequest::SyncCollection(sync_collection) => {
             handle_sync_collection(
                 sync_collection,
+                &props,
                 req,
                 &user,
                 &principal,
@@ -64,7 +89,7 @@ pub async fn route_report_addressbook<AS: AddressbookStore>(
 
 #[cfg(test)]
 mod tests {
-    use rustical_dav::xml::{sync_collection::SyncLevel, PropElement, Propname};
+    use rustical_dav::xml::{PropElement, Propname, sync_collection::SyncLevel};
 
     use super::*;
 

crates/carddav/src/addressbook/methods/report/sync_collection.rs

@@ -6,8 +6,7 @@ use actix_web::{HttpRequest, http::StatusCode};
 use rustical_dav::{
     resource::Resource,
     xml::{
-        MultistatusElement, PropElement, PropfindType, multistatus::ResponseElement,
+        MultistatusElement, multistatus::ResponseElement, sync_collection::SyncCollectionRequest,
-        sync_collection::SyncCollectionRequest,
     },
 };
 use rustical_store::{
@@ -17,26 +16,14 @@ use rustical_store::{
 };
 
 pub async fn handle_sync_collection<AS: AddressbookStore>(
-    sync_collection: SyncCollectionRequest,
+    sync_collection: &SyncCollectionRequest,
+    props: &[&str],
     req: HttpRequest,
     user: &User,
     principal: &str,
     addressbook_id: &str,
     addr_store: &AS,
 ) -> Result<MultistatusElement<AddressObjectPropWrapper, String>, Error> {
-    let props = match sync_collection.prop {
-        PropfindType::Allprop => {
-            vec!["allprop".to_owned()]
-        }
-        PropfindType::Propname => {
-            vec!["propname".to_owned()]
-        }
-        PropfindType::Prop(PropElement(prop_tags)) => {
-            prop_tags.into_iter().map(|propname| propname.0).collect()
-        }
-    };
-    let props: Vec<&str> = props.iter().map(String::as_str).collect();
-
     let old_synctoken = parse_synctoken(&sync_collection.sync_token).unwrap_or(0);
     let (new_objects, deleted_objects, new_synctoken) = addr_store
         .sync_changes(principal, addressbook_id, old_synctoken)
@@ -54,7 +41,7 @@ pub async fn handle_sync_collection<AS: AddressbookStore>(
                 object,
                 principal: principal.to_owned(),
             }
-            .propfind(&path, &props, user, req.resource_map())?,
+            .propfind(&path, props, user, req.resource_map())?,
         );
     }
 

crates/store/src/calendar/timestamp.rs

@@ -1,3 +1,4 @@
+use super::IcalProperty;
 use crate::Error;
 use chrono::{DateTime, Duration, NaiveDate, NaiveDateTime, NaiveTime, Utc};
 use chrono_tz::Tz;
@@ -10,8 +11,6 @@ use lazy_static::lazy_static;
 use rustical_xml::{ValueDeserialize, ValueSerialize};
 use std::{collections::HashMap, ops::Add};
 
-use super::IcalProperty;
-
 lazy_static! {
     static ref RE_DURATION: regex::Regex = regex::Regex::new(r"^(?<sign>[+-])?P((?P<W>\d+)W)?((?P<D>\d+)D)?(T((?P<H>\d+)H)?((?P<M>\d+)M)?((?P<S>\d+)S)?)?$").unwrap();
 
@@ -40,6 +39,7 @@ impl ValueDeserialize for UtcDateTime {
         ))
     }
 }
+
 impl ValueSerialize for UtcDateTime {
     fn serialize(&self) -> String {
         format!("{}", self.0.format(UTC_DATE_TIME))
@@ -96,7 +96,7 @@ impl CalDateTime {
                         Some(tz)
                     } else {
                         return Err(Error::InvalidData(format!(
-                            "Timezone has X-LIC-LOCATION property to specify a timezone from the Olson database, however it's value {olson_name} is invalid"
+                            "Timezone has X-LIC-LOCATION property to specify a timezone from the Olson database, however its value {olson_name} is invalid"
                         )));
                     }
                 } else {
@@ -109,7 +109,7 @@ impl CalDateTime {
             } else {
                 // TZID refers to timezone that does not exist
                 return Err(Error::InvalidData(format!(
-                    "No timezone specified with TZID={tzid}"
+                    "Timezone {tzid} does not exist"
                 )));
             }
         } else {