Add <remove> to PROPPATCH implementation and some refactoring

README.md

@@ -9,7 +9,7 @@ a calendar server
   - [ ] Proper filtering for REPORT method
   - [ ] ICS parsing
     - [x] Datetime parsing
-  - [ ] Implement PROPPATCH
+  - [x] Implement PROPPATCH
 - [ ] Access Control
   - [ ] OIDC support
 - [ ] CardDAV
@@ -22,4 +22,3 @@ a calendar server
 - [x] Trash bin
   - [x] Hiding calendars instead of deleting them
   - [ ] Restore endpoint
-

crates/caldav/src/calendar/resource.rs

@@ -272,6 +272,41 @@ impl Resource for CalendarFile {
         }
     }
 
+    fn remove_prop(&mut self, prop: Self::PropName) -> Result<(), rustical_dav::Error> {
+        match prop {
+            CalendarPropName::Resourcetype => Err(rustical_dav::Error::PropReadOnly),
+            CalendarPropName::CurrentUserPrincipal => Err(rustical_dav::Error::PropReadOnly),
+            CalendarPropName::Owner => Err(rustical_dav::Error::PropReadOnly),
+            CalendarPropName::Displayname => {
+                self.calendar.displayname = None;
+                Ok(())
+            }
+            CalendarPropName::CalendarColor => {
+                self.calendar.color = None;
+                Ok(())
+            }
+            CalendarPropName::CalendarDescription => {
+                self.calendar.description = None;
+                Ok(())
+            }
+            CalendarPropName::CalendarTimezone => {
+                self.calendar.timezone = None;
+                Ok(())
+            }
+            CalendarPropName::CalendarOrder => {
+                self.calendar.order = 0;
+                Ok(())
+            }
+            CalendarPropName::SupportedCalendarComponentSet => {
+                Err(rustical_dav::Error::PropReadOnly)
+            }
+            CalendarPropName::SupportedCalendarData => Err(rustical_dav::Error::PropReadOnly),
+            CalendarPropName::Getcontenttype => Err(rustical_dav::Error::PropReadOnly),
+            CalendarPropName::MaxResourceSize => Err(rustical_dav::Error::PropReadOnly),
+            CalendarPropName::CurrentUserPrivilegeSet => Err(rustical_dav::Error::PropReadOnly),
+        }
+    }
+
     fn get_path(&self) -> &str {
         &self.path
     }

crates/caldav/src/event/resource.rs

@@ -70,10 +70,6 @@ impl Resource for EventFile {
             )),
         }
     }
-
-    fn set_prop(&mut self, _prop: Self::Prop) -> Result<(), rustical_dav::Error> {
-        Err(rustical_dav::Error::PropReadOnly)
-    }
 }
 
 #[async_trait(?Send)]

crates/caldav/src/principal/mod.rs

@@ -91,10 +91,6 @@ impl Resource for PrincipalFile {
         }
     }
 
-    fn set_prop(&mut self, _prop: Self::Prop) -> Result<(), rustical_dav::Error> {
-        Err(rustical_dav::Error::PropReadOnly)
-    }
-
     fn get_path(&self) -> &str {
         &self.path
     }

crates/caldav/src/root/mod.rs

@@ -60,10 +60,6 @@ impl Resource for RootFile {
         }
     }
 
-    fn set_prop(&mut self, _prop: Self::Prop) -> Result<(), rustical_dav::Error> {
-        Err(rustical_dav::Error::PropReadOnly)
-    }
-
     fn get_path(&self) -> &str {
         &self.path
     }

crates/dav/src/proppatch.rs

@@ -1,3 +1,5 @@
+use std::str::FromStr;
+
 use crate::namespace::Namespace;
 use crate::resource::InvalidProperty;
 use crate::resource::Resource;
@@ -31,17 +33,21 @@ struct SetPropertyElement<T> {
 #[derive(Deserialize, Clone, Debug)]
 #[serde(rename_all = "kebab-case")]
 struct RemovePropertyElement {
-    #[serde(rename = "$value")]
+    prop: PropertyElement<TagName>,
-    prop: TagName,
+}
+
+#[derive(Deserialize, Clone, Debug)]
+#[serde(rename_all = "kebab-case")]
+enum Operation<T> {
+    Set(SetPropertyElement<T>),
+    Remove(RemovePropertyElement),
 }
 
 #[derive(Deserialize, Clone, Debug)]
 #[serde(rename_all = "kebab-case")]
 struct PropertyupdateElement<T> {
-    #[serde(default = "Vec::new")]
+    #[serde(rename = "$value", default = "Vec::new")]
-    set: Vec<SetPropertyElement<T>>,
+    operations: Vec<Operation<T>>,
-    #[serde(default = "Vec::new")]
-    remove: Vec<RemovePropertyElement>,
 }
 
 pub async fn route_proppatch<A: CheckAuthentication, R: ResourceService + ?Sized>(
@@ -57,74 +63,73 @@ pub async fn route_proppatch<A: CheckAuthentication, R: ResourceService + ?Sized
 
     debug!("{body}");
 
-    // TODO: Implement remove!
+    let PropertyupdateElement::<<R::File as Resource>::Prop> { operations } =
-    let PropertyupdateElement::<<R::File as Resource>::Prop> {
+        quick_xml::de::from_str(&body).map_err(Error::XmlDecodeError)?;
-        set: set_els,
-        remove: remove_els,
-    } = quick_xml::de::from_str(&body).map_err(Error::XmlDecodeError)?;
 
-    // Extract all property names without verification
+    // Extract all set property names without verification
     // Weird workaround because quick_xml doesn't allow untagged enums
     let propnames: Vec<String> = quick_xml::de::from_str::<PropertyupdateElement<TagName>>(&body)
         .map_err(Error::XmlDecodeError)?
-        .set
+        .operations
         .into_iter()
-        .map(|set_el| set_el.prop.prop.into())
+        .map(|op_el| match op_el {
-        .collect();
+            Operation::Set(set_el) => set_el.prop.prop.into(),
-
+            // If we can't remove a nonexisting property then that's no big deal
-    // Invalid properties
+            Operation::Remove(remove_el) => remove_el.prop.prop.into(),
-    let props_not_found: Vec<String> = propnames
+        })
-        .iter()
-        .zip(&set_els)
-        .filter_map(
-            |(
-                name,
-                SetPropertyElement {
-                    prop: PropertyElement { prop },
-                },
-            )| {
-                if prop.invalid_property() {
-                    Some(name.to_string())
-                } else {
-                    None
-                }
-            },
-        )
-        .collect();
-
-    // Filter out invalid props
-    let set_props: Vec<<R::File as Resource>::Prop> = set_els
-        .into_iter()
-        .filter_map(
-            |SetPropertyElement {
-                 prop: PropertyElement { prop },
-             }| {
-                if prop.invalid_property() {
-                    None
-                } else {
-                    Some(prop)
-                }
-            },
-        )
         .collect();
 
     let mut resource = resource_service.get_file().await?;
 
     let mut props_ok = Vec::new();
     let mut props_conflict = Vec::new();
+    let mut props_not_found = Vec::new();
 
-    for (prop, propname) in set_props.into_iter().zip(propnames) {
+    for (operation, propname) in operations.into_iter().zip(propnames) {
-        match resource.set_prop(prop) {
+        match operation {
-            Ok(()) => {
+            Operation::Set(SetPropertyElement {
-                props_ok.push(propname);
+                prop: PropertyElement { prop },
+            }) => {
+                if prop.invalid_property() {
+                    props_not_found.push(propname);
+                    continue;
+                }
+                match resource.set_prop(prop) {
+                    Ok(()) => {
+                        props_ok.push(propname);
+                    }
+                    Err(Error::PropReadOnly) => {
+                        props_conflict.push(propname);
+                    }
+                    Err(err) => {
+                        // TODO: Think about error handling?
+                        return Err(err.into());
+                    }
+                }
             }
-            Err(Error::PropReadOnly) => {
+            Operation::Remove(_remove_el) => {
-                props_conflict.push(propname);
+                match <<R::File as Resource>::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) => {
+                                // TODO: Think about error handling?
+                                return Err(err.into());
+                            }
+                        }
+                    }
+                    Err(_) => {
+                        // I guess removing a nonexisting property should be successful :)
+                        props_ok.push(propname);
+                    }
+                };
             }
-            Err(err) => {
+        }
-                return Err(err.into());
-            }
-        };
     }
 
     if props_not_found.is_empty() && props_conflict.is_empty() {

crates/dav/src/resource.rs

@@ -23,7 +23,13 @@ pub trait Resource: Clone {
 
     fn get_path(&self) -> &str;
 
-    fn set_prop(&mut self, prop: Self::Prop) -> Result<(), crate::Error>;
+    fn set_prop(&mut self, _prop: Self::Prop) -> Result<(), crate::Error> {
+        Err(crate::Error::PropReadOnly)
+    }
+
+    fn remove_prop(&mut self, _prop: Self::PropName) -> Result<(), crate::Error> {
+        Err(crate::Error::PropReadOnly)
+    }
 }
 
 pub trait InvalidProperty {