From c21993ab15bd0cd0de20978136951f35b4341a70 Mon Sep 17 00:00:00 2001 From: Lennart <18233294+lennart-k@users.noreply.github.com> Date: Wed, 6 Nov 2024 15:57:10 +0100 Subject: [PATCH] Refactoring --- .../methods/report/calendar_multiget.rs | 15 ++----- .../calendar/methods/report/calendar_query.rs | 12 ++---- .../methods/report/sync_collection.rs | 16 ++------ .../methods/report/addressbook_multiget.rs | 16 ++------ .../methods/report/sync_collection.rs | 16 ++------ crates/carddav/tests/integration_test.rs | 11 +++++ crates/dav/src/lib.rs | 1 - crates/dav/src/methods/mod.rs | 7 ---- .../dav/src/{ => resource}/methods/delete.rs | 0 crates/dav/src/resource/methods/mod.rs | 7 ++++ .../src/{ => resource}/methods/propfind.rs | 34 ++++----------- .../src/{ => resource}/methods/proppatch.rs | 41 +++++++------------ crates/dav/src/resource/mod.rs | 6 +-- crates/dav/src/resource/resource_service.rs | 3 +- crates/dav/src/xml/mod.rs | 3 ++ crates/dav/src/xml/multistatus.rs | 8 ++-- crates/dav/src/xml/propfind.rs | 17 ++++++++ 17 files changed, 85 insertions(+), 128 deletions(-) create mode 100644 crates/carddav/tests/integration_test.rs delete mode 100644 crates/dav/src/methods/mod.rs rename crates/dav/src/{ => resource}/methods/delete.rs (100%) create mode 100644 crates/dav/src/resource/methods/mod.rs rename crates/dav/src/{ => resource}/methods/propfind.rs (74%) rename crates/dav/src/{ => resource}/methods/proppatch.rs (81%) create mode 100644 crates/dav/src/xml/propfind.rs diff --git a/crates/caldav/src/calendar/methods/report/calendar_multiget.rs b/crates/caldav/src/calendar/methods/report/calendar_multiget.rs index 47ffd71..bcf750b 100644 --- a/crates/caldav/src/calendar/methods/report/calendar_multiget.rs +++ b/crates/caldav/src/calendar/methods/report/calendar_multiget.rs @@ -9,12 +9,8 @@ use actix_web::{ HttpRequest, }; use rustical_dav::{ - methods::propfind::{PropElement, PropfindType}, resource::{CommonPropertiesProp, EitherProp, Resource}, - xml::{ - multistatus::{PropstatWrapper, ResponseElement}, - MultistatusElement, - }, + xml::{multistatus::ResponseElement, MultistatusElement, PropElement, PropfindType}, }; use rustical_store::{auth::User, CalendarObject, CalendarStore}; use serde::Deserialize; @@ -69,13 +65,8 @@ pub async fn handle_calendar_multiget( principal: &str, cal_id: &str, cal_store: &C, -) -> Result< - MultistatusElement< - PropstatWrapper>, - String, - >, - Error, -> { +) -> Result, String>, Error> +{ let principal_url = PrincipalResource::get_url(req.resource_map(), vec![principal]).unwrap(); let (objects, not_found) = get_objects_calendar_multiget(&cal_multiget, &principal_url, principal, cal_id, cal_store) diff --git a/crates/caldav/src/calendar/methods/report/calendar_query.rs b/crates/caldav/src/calendar/methods/report/calendar_query.rs index 906a288..a55137e 100644 --- a/crates/caldav/src/calendar/methods/report/calendar_query.rs +++ b/crates/caldav/src/calendar/methods/report/calendar_query.rs @@ -1,9 +1,8 @@ use actix_web::HttpRequest; use chrono::{DateTime, Utc}; use rustical_dav::{ - methods::propfind::{PropElement, PropfindType}, resource::{CommonPropertiesProp, EitherProp, Resource}, - xml::{multistatus::PropstatWrapper, MultistatusElement}, + xml::{MultistatusElement, PropElement, PropfindType}, }; use rustical_store::{auth::User, CalendarObject, CalendarStore}; use serde::Deserialize; @@ -210,13 +209,8 @@ pub async fn handle_calendar_query( principal: &str, cal_id: &str, cal_store: &C, -) -> Result< - MultistatusElement< - PropstatWrapper>, - String, - >, - Error, -> { +) -> Result, String>, Error> +{ let objects = get_objects_calendar_query(&cal_query, principal, cal_id, cal_store).await?; let props = match cal_query.prop { diff --git a/crates/caldav/src/calendar/methods/report/sync_collection.rs b/crates/caldav/src/calendar/methods/report/sync_collection.rs index 48e3ff5..3c60ccf 100644 --- a/crates/caldav/src/calendar/methods/report/sync_collection.rs +++ b/crates/caldav/src/calendar/methods/report/sync_collection.rs @@ -1,11 +1,8 @@ use actix_web::{http::StatusCode, HttpRequest}; use rustical_dav::{ - methods::propfind::{PropElement, PropfindType}, resource::{CommonPropertiesProp, EitherProp, Resource}, - xml::{ - multistatus::{PropstatWrapper, ResponseElement}, - MultistatusElement, - }, + xml::{multistatus::ResponseElement, MultistatusElement}, + xml::{PropElement, PropfindType}, }; use rustical_store::{ auth::User, @@ -49,13 +46,8 @@ pub async fn handle_sync_collection( principal: &str, cal_id: &str, cal_store: &C, -) -> Result< - MultistatusElement< - PropstatWrapper>, - String, - >, - Error, -> { +) -> Result, String>, Error> +{ let props = match sync_collection.prop { PropfindType::Allprop => { vec!["allprop".to_owned()] diff --git a/crates/carddav/src/addressbook/methods/report/addressbook_multiget.rs b/crates/carddav/src/addressbook/methods/report/addressbook_multiget.rs index 570e368..2fc75c0 100644 --- a/crates/carddav/src/addressbook/methods/report/addressbook_multiget.rs +++ b/crates/carddav/src/addressbook/methods/report/addressbook_multiget.rs @@ -9,12 +9,9 @@ use actix_web::{ HttpRequest, }; use rustical_dav::{ - methods::propfind::{PropElement, PropfindType}, resource::{CommonPropertiesProp, EitherProp, Resource}, - xml::{ - multistatus::{PropstatWrapper, ResponseElement}, - MultistatusElement, - }, + xml::{multistatus::ResponseElement, MultistatusElement}, + xml::{PropElement, PropfindType}, }; use rustical_store::{auth::User, AddressObject, AddressbookStore}; use serde::Deserialize; @@ -68,13 +65,8 @@ pub async fn handle_addressbook_multiget( principal: &str, cal_id: &str, addr_store: &AS, -) -> Result< - MultistatusElement< - PropstatWrapper>, - String, - >, - Error, -> { +) -> Result, String>, Error> +{ let principal_url = PrincipalResource::get_url(req.resource_map(), vec![principal]).unwrap(); let (objects, not_found) = get_objects_addressbook_multiget( &addr_multiget, diff --git a/crates/carddav/src/addressbook/methods/report/sync_collection.rs b/crates/carddav/src/addressbook/methods/report/sync_collection.rs index 923e561..138595b 100644 --- a/crates/carddav/src/addressbook/methods/report/sync_collection.rs +++ b/crates/carddav/src/addressbook/methods/report/sync_collection.rs @@ -4,12 +4,9 @@ use crate::{ }; use actix_web::{http::StatusCode, HttpRequest}; use rustical_dav::{ - methods::propfind::{PropElement, PropfindType}, resource::{CommonPropertiesProp, EitherProp, Resource}, - xml::{ - multistatus::{PropstatWrapper, ResponseElement}, - MultistatusElement, - }, + xml::{multistatus::ResponseElement, MultistatusElement}, + xml::{PropElement, PropfindType}, }; use rustical_store::{ auth::User, @@ -47,13 +44,8 @@ pub async fn handle_sync_collection( principal: &str, addressbook_id: &str, addr_store: &AS, -) -> Result< - MultistatusElement< - PropstatWrapper>, - String, - >, - Error, -> { +) -> Result, String>, Error> +{ let props = match sync_collection.prop { PropfindType::Allprop => { vec!["allprop".to_owned()] diff --git a/crates/carddav/tests/integration_test.rs b/crates/carddav/tests/integration_test.rs new file mode 100644 index 0000000..42cf6a9 --- /dev/null +++ b/crates/carddav/tests/integration_test.rs @@ -0,0 +1,11 @@ +use actix_web::{web, App}; +use rustical_carddav::configure_dav; + +#[actix_web::test] +async fn test_asd() { + let app = App::new().service(web::scope("/carddav").configure( + |config| configure_dav(config, auth_provider, store) + )); + let app = actix_web::test::init_service() + assert!(false); +} diff --git a/crates/dav/src/lib.rs b/crates/dav/src/lib.rs index 4884ea4..fa87230 100644 --- a/crates/dav/src/lib.rs +++ b/crates/dav/src/lib.rs @@ -1,6 +1,5 @@ pub mod depth_header; pub mod error; -pub mod methods; pub mod namespace; pub mod privileges; pub mod resource; diff --git a/crates/dav/src/methods/mod.rs b/crates/dav/src/methods/mod.rs deleted file mode 100644 index 5567d67..0000000 --- a/crates/dav/src/methods/mod.rs +++ /dev/null @@ -1,7 +0,0 @@ -pub mod delete; -pub mod propfind; -pub mod proppatch; - -pub use delete::route_delete; -pub use propfind::route_propfind; -pub use proppatch::route_proppatch; diff --git a/crates/dav/src/methods/delete.rs b/crates/dav/src/resource/methods/delete.rs similarity index 100% rename from crates/dav/src/methods/delete.rs rename to crates/dav/src/resource/methods/delete.rs diff --git a/crates/dav/src/resource/methods/mod.rs b/crates/dav/src/resource/methods/mod.rs new file mode 100644 index 0000000..5d1b389 --- /dev/null +++ b/crates/dav/src/resource/methods/mod.rs @@ -0,0 +1,7 @@ +mod delete; +mod propfind; +mod proppatch; + +pub(crate) use delete::route_delete; +pub(crate) use propfind::route_propfind; +pub(crate) use proppatch::route_proppatch; diff --git a/crates/dav/src/methods/propfind.rs b/crates/dav/src/resource/methods/propfind.rs similarity index 74% rename from crates/dav/src/methods/propfind.rs rename to crates/dav/src/resource/methods/propfind.rs index cee2c98..4550d24 100644 --- a/crates/dav/src/methods/propfind.rs +++ b/crates/dav/src/resource/methods/propfind.rs @@ -4,9 +4,9 @@ use crate::resource::CommonPropertiesProp; use crate::resource::EitherProp; use crate::resource::Resource; use crate::resource::ResourceService; -use crate::xml::multistatus::PropstatWrapper; use crate::xml::MultistatusElement; -use crate::xml::TagList; +use crate::xml::PropElement; +use crate::xml::PropfindType; use crate::Error; use actix_web::web::Path; use actix_web::HttpRequest; @@ -17,21 +17,7 @@ use tracing_actix_web::RootSpan; #[derive(Deserialize, Clone, Debug)] #[serde(rename_all = "kebab-case")] -pub struct PropElement { - #[serde(flatten)] - pub prop: TagList, -} - -#[derive(Deserialize, Clone, Debug)] -#[serde(rename_all = "kebab-case")] -pub enum PropfindType { - Propname, - Allprop, - Prop(PropElement), -} - -#[derive(Deserialize, Clone, Debug)] -#[serde(rename_all = "kebab-case")] +#[serde(deny_unknown_fields)] struct PropfindElement { #[serde(rename = "$value")] prop: PropfindType, @@ -39,7 +25,7 @@ struct PropfindElement { #[instrument(parent = root_span.id(), skip(path_components, req, root_span))] #[allow(clippy::type_complexity)] -pub async fn route_propfind( +pub(crate) async fn route_propfind( path_components: Path, body: String, req: HttpRequest, @@ -48,8 +34,8 @@ pub async fn route_propfind( root_span: RootSpan, ) -> Result< MultistatusElement< - PropstatWrapper::Prop, CommonPropertiesProp>>, - PropstatWrapper::Prop, CommonPropertiesProp>>, + EitherProp<::Prop, CommonPropertiesProp>, + EitherProp<::Prop, CommonPropertiesProp>, >, R::Error, > { @@ -71,12 +57,8 @@ pub async fn route_propfind( }; let props = match propfind.prop { - PropfindType::Allprop => { - vec!["allprop".to_owned()] - } - PropfindType::Propname => { - vec!["propname".to_owned()] - } + PropfindType::Allprop => vec!["allprop".to_owned()], + PropfindType::Propname => vec!["propname".to_owned()], PropfindType::Prop(PropElement { prop: prop_tags }) => prop_tags.into_inner(), }; let props: Vec<&str> = props.iter().map(String::as_str).collect(); diff --git a/crates/dav/src/methods/proppatch.rs b/crates/dav/src/resource/methods/proppatch.rs similarity index 81% rename from crates/dav/src/methods/proppatch.rs rename to crates/dav/src/resource/methods/proppatch.rs index a041eb0..d8c5f15 100644 --- a/crates/dav/src/methods/proppatch.rs +++ b/crates/dav/src/resource/methods/proppatch.rs @@ -37,8 +37,8 @@ struct RemovePropertyElement { #[derive(Deserialize, Clone, Debug)] #[serde(rename_all = "kebab-case")] -enum Operation { - Set(SetPropertyElement), +enum Operation { + Set(SetPropertyElement), Remove(RemovePropertyElement), } @@ -50,17 +50,18 @@ struct PropertyupdateElement { } #[instrument(parent = root_span.id(), skip(path, req, root_span))] -pub async fn route_proppatch( +pub(crate) async fn route_proppatch( path: Path, body: String, req: HttpRequest, user: User, root_span: RootSpan, -) -> Result, PropstatWrapper>, R::Error> { +) -> Result, R::Error> { let path_components = path.into_inner(); let href = req.path().to_owned(); let resource_service = R::new(&req, path_components.clone()).await?; + // Extract operations let PropertyupdateElement::<::Prop> { operations } = quick_xml::de::from_str(&body).map_err(Error::XmlDeserializationError)?; @@ -104,34 +105,20 @@ pub async fn route_proppatch( continue; } match resource.set_prop(prop) { - Ok(()) => { - props_ok.push(propname); - } - Err(Error::PropReadOnly) => { - props_conflict.push(propname); - } - Err(err) => { - return Err(err.into()); - } - } + Ok(()) => props_ok.push(propname), + Err(Error::PropReadOnly) => props_conflict.push(propname), + Err(err) => return Err(err.into()), + }; } Operation::Remove(_remove_el) => { match <::PropName as FromStr>::from_str(&propname) { Ok(prop) => match resource.remove_prop(&prop) { - Ok(()) => { - props_ok.push(propname); - } - Err(Error::PropReadOnly) => { - props_conflict.push(propname); - } - Err(err) => { - return Err(err.into()); - } + Ok(()) => props_ok.push(propname), + Err(Error::PropReadOnly) => props_conflict.push(propname), + Err(err) => return Err(err.into()), }, - Err(_) => { - // I guess removing a nonexisting property should be successful :) - props_ok.push(propname); - } + // I guess removing a nonexisting property should be successful :) + Err(_) => props_ok.push(propname), }; } } diff --git a/crates/dav/src/resource/mod.rs b/crates/dav/src/resource/mod.rs index 0b88496..6794212 100644 --- a/crates/dav/src/resource/mod.rs +++ b/crates/dav/src/resource/mod.rs @@ -16,6 +16,7 @@ use std::str::FromStr; use strum::{EnumString, VariantNames}; mod invalid_property; +mod methods; mod resource_service; pub trait ResourceProp: InvalidProperty + Serialize + for<'de> Deserialize<'de> {} @@ -149,10 +150,7 @@ pub trait Resource: Clone + 'static { mut props: Vec<&str>, user: &User, rmap: &ResourceMap, - ) -> Result< - ResponseElement>>, - Self::Error, - > { + ) -> Result>, Self::Error> { if props.contains(&"propname") { if props.len() != 1 { // propname MUST be the only queried prop per spec diff --git a/crates/dav/src/resource/resource_service.rs b/crates/dav/src/resource/resource_service.rs index 2fde81a..643b126 100644 --- a/crates/dav/src/resource/resource_service.rs +++ b/crates/dav/src/resource/resource_service.rs @@ -4,8 +4,7 @@ use actix_web::{dev::ResourceMap, http::Method, web, HttpRequest, ResponseError} use async_trait::async_trait; use serde::Deserialize; -use crate::methods::{route_delete, route_propfind, route_proppatch}; - +use super::methods::{route_delete, route_propfind, route_proppatch}; use super::Resource; #[async_trait(?Send)] diff --git a/crates/dav/src/xml/mod.rs b/crates/dav/src/xml/mod.rs index 122a253..46dbbe6 100644 --- a/crates/dav/src/xml/mod.rs +++ b/crates/dav/src/xml/mod.rs @@ -1,8 +1,11 @@ pub mod multistatus; +mod propfind; mod resourcetype; pub mod tag_list; pub mod tag_name; +pub use propfind::{PropElement, PropfindType}; + use derive_more::derive::From; pub use multistatus::MultistatusElement; pub use tag_list::TagList; diff --git a/crates/dav/src/xml/multistatus.rs b/crates/dav/src/xml/multistatus.rs index bc05409..0bf9ec8 100644 --- a/crates/dav/src/xml/multistatus.rs +++ b/crates/dav/src/xml/multistatus.rs @@ -37,7 +37,7 @@ pub struct ResponseElement { pub href: String, #[serde(skip_serializing_if = "Option::is_none")] pub status: Option, - pub propstat: Vec, + pub propstat: Vec>, } impl Default for ResponseElement { @@ -55,11 +55,11 @@ impl Default for ResponseElement { // Extended by sync-token as specified in RFC 6578 #[derive(Serialize)] #[serde(rename = "multistatus", rename_all = "kebab-case")] -pub struct MultistatusElement { +pub struct MultistatusElement { #[serde(rename = "response")] - pub responses: Vec>, + pub responses: Vec>, #[serde(rename = "response")] - pub member_responses: Vec>, + pub member_responses: Vec>, #[serde(rename = "@xmlns")] pub ns_dav: &'static str, #[serde(rename = "@xmlns:C")] diff --git a/crates/dav/src/xml/propfind.rs b/crates/dav/src/xml/propfind.rs new file mode 100644 index 0000000..0f9441b --- /dev/null +++ b/crates/dav/src/xml/propfind.rs @@ -0,0 +1,17 @@ +use super::TagList; +use serde::Deserialize; + +#[derive(Deserialize, Clone, Debug)] +#[serde(rename_all = "kebab-case")] +pub struct PropElement { + #[serde(flatten)] + pub prop: TagList, +} + +#[derive(Deserialize, Clone, Debug)] +#[serde(rename_all = "kebab-case")] +pub enum PropfindType { + Propname, + Allprop, + Prop(PropElement), +}