Remove RwLock around stores, locking shall be the responsibility of the store implementation

crates/caldav/src/calendar/methods/mkcalendar.rs

@@ -5,7 +5,6 @@ use rustical_store::auth::User;
 use rustical_store::model::Calendar;
 use rustical_store::CalendarStore;
 use serde::{Deserialize, Serialize};
-use tokio::sync::RwLock;
 
 #[derive(Deserialize, Serialize, Clone, Debug)]
 #[serde(rename_all = "kebab-case")]
@@ -57,7 +56,7 @@ pub async fn route_mkcalendar<C: CalendarStore + ?Sized>(
     path: Path<(String, String)>,
     body: String,
     user: User,
-    store: Data<RwLock<C>>,
+    store: Data<C>,
 ) -> Result<HttpResponse, Error> {
     let (principal, cal_id) = path.into_inner();
     if principal != user.id {
@@ -79,7 +78,7 @@ pub async fn route_mkcalendar<C: CalendarStore + ?Sized>(
         synctoken: 0,
     };
 
-    match store.read().await.get_calendar(&principal, &cal_id).await {
+    match store.get_calendar(&principal, &cal_id).await {
         Err(rustical_store::Error::NotFound) => {
             // No conflict, no worries
         }
@@ -93,7 +92,7 @@ pub async fn route_mkcalendar<C: CalendarStore + ?Sized>(
         }
     }
 
-    match store.write().await.insert_calendar(calendar).await {
+    match store.insert_calendar(calendar).await {
         // The spec says we should return a mkcalendar-response but I don't know what goes into it.
         // However, it works without one but breaks on iPadOS when using an empty one :)
         Ok(()) => Ok(HttpResponse::Created()

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

@@ -18,7 +18,6 @@ use rustical_dav::{
 };
 use rustical_store::{model::object::CalendarObject, CalendarStore};
 use serde::Deserialize;
-use tokio::sync::RwLock;
 
 #[derive(Deserialize, Clone, Debug)]
 #[serde(rename_all = "kebab-case")]
@@ -35,7 +34,7 @@ pub async fn get_objects_calendar_multiget<C: CalendarStore + ?Sized>(
     principal_url: &str,
     principal: &str,
     cal_id: &str,
-    store: &RwLock<C>,
+    store: &C,
 ) -> Result<(Vec<CalendarObject>, Vec<String>), Error> {
     let resource_def =
         ResourceDef::prefix(principal_url).join(&ResourceDef::new("/{cal_id}/{object_id}"));
@@ -43,7 +42,6 @@ pub async fn get_objects_calendar_multiget<C: CalendarStore + ?Sized>(
     let mut result = vec![];
     let mut not_found = vec![];
 
-    let store = store.read().await;
     for href in &cal_query.href {
         let mut path = Path::new(href.as_str());
         if !resource_def.capture_match_info(&mut path) {
@@ -69,7 +67,7 @@ pub async fn handle_calendar_multiget<C: CalendarStore + ?Sized>(
     req: HttpRequest,
     principal: &str,
     cal_id: &str,
-    cal_store: &RwLock<C>,
+    cal_store: &C,
 ) -> Result<MultistatusElement<PropstatWrapper<CalendarObjectProp>, String>, Error> {
     let principal_url = PrincipalResource::get_url(req.resource_map(), vec![principal]).unwrap();
     let (objects, not_found) =

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

@@ -7,7 +7,6 @@ use rustical_dav::{
 };
 use rustical_store::{model::object::CalendarObject, CalendarStore};
 use serde::Deserialize;
-use tokio::sync::RwLock;
 
 use crate::{
     calendar_object::resource::{CalendarObjectProp, CalendarObjectResource},
@@ -195,9 +194,9 @@ pub async fn get_objects_calendar_query<C: CalendarStore + ?Sized>(
     cal_query: &CalendarQueryRequest,
     principal: &str,
     cal_id: &str,
-    store: &RwLock<C>,
+    store: &C,
 ) -> Result<Vec<CalendarObject>, Error> {
-    let mut objects = store.read().await.get_objects(principal, cal_id).await?;
+    let mut objects = store.get_objects(principal, cal_id).await?;
     if let Some(filter) = &cal_query.filter {
         objects.retain(|object| filter.matches(object));
     }
@@ -209,7 +208,7 @@ pub async fn handle_calendar_query<C: CalendarStore + ?Sized>(
     req: HttpRequest,
     principal: &str,
     cal_id: &str,
-    cal_store: &RwLock<C>,
+    cal_store: &C,
 ) -> Result<MultistatusElement<PropstatWrapper<CalendarObjectProp>, String>, Error> {
     let objects = get_objects_calendar_query(&cal_query, principal, cal_id, cal_store).await?;
 

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

@@ -8,7 +8,6 @@ use calendar_query::{handle_calendar_query, CalendarQueryRequest};
 use rustical_store::{auth::User, CalendarStore};
 use serde::{Deserialize, Serialize};
 use sync_collection::{handle_sync_collection, SyncCollectionRequest};
-use tokio::sync::RwLock;
 use tracing::instrument;
 
 mod calendar_multiget;
@@ -37,7 +36,7 @@ pub async fn route_report_calendar<C: CalendarStore + ?Sized>(
     body: String,
     user: User,
     req: HttpRequest,
-    cal_store: Data<RwLock<C>>,
+    cal_store: Data<C>,
 ) -> Result<impl Responder, Error> {
     let (principal, cal_id) = path.into_inner();
     if principal != user.id {
@@ -48,13 +47,21 @@ pub async fn route_report_calendar<C: CalendarStore + ?Sized>(
 
     Ok(match request.clone() {
         ReportRequest::CalendarQuery(cal_query) => {
-            handle_calendar_query(cal_query, req, &principal, &cal_id, &cal_store).await?
+            handle_calendar_query(cal_query, req, &principal, &cal_id, cal_store.as_ref()).await?
         }
         ReportRequest::CalendarMultiget(cal_multiget) => {
-            handle_calendar_multiget(cal_multiget, req, &principal, &cal_id, &cal_store).await?
+            handle_calendar_multiget(cal_multiget, req, &principal, &cal_id, cal_store.as_ref())
+                .await?
         }
         ReportRequest::SyncCollection(sync_collection) => {
-            handle_sync_collection(sync_collection, req, &principal, &cal_id, &cal_store).await?
+            handle_sync_collection(
+                sync_collection,
+                req,
+                &principal,
+                &cal_id,
+                cal_store.as_ref(),
+            )
+            .await?
         }
     })
 }

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

@@ -12,7 +12,6 @@ use rustical_store::{
     CalendarStore,
 };
 use serde::Deserialize;
-use tokio::sync::RwLock;
 
 use crate::{
     calendar_object::resource::{CalendarObjectProp, CalendarObjectResource},
@@ -47,7 +46,7 @@ pub async fn handle_sync_collection<C: CalendarStore + ?Sized>(
     req: HttpRequest,
     principal: &str,
     cal_id: &str,
-    cal_store: &RwLock<C>,
+    cal_store: &C,
 ) -> Result<MultistatusElement<PropstatWrapper<CalendarObjectProp>, String>, Error> {
     let props = match sync_collection.prop {
         PropfindType::Allprop => {
@@ -62,8 +61,6 @@ pub async fn handle_sync_collection<C: CalendarStore + ?Sized>(
 
     let old_synctoken = parse_synctoken(&sync_collection.sync_token).unwrap_or(0);
     let (new_objects, deleted_objects, new_synctoken) = cal_store
-        .read()
-        .await
         .sync_changes(principal, cal_id, old_synctoken)
         .await?;
 

crates/caldav/src/calendar/resource.rs

@@ -21,10 +21,9 @@ use serde::{Deserialize, Serialize};
 use std::str::FromStr;
 use std::sync::Arc;
 use strum::{EnumString, VariantNames};
-use tokio::sync::RwLock;
 
 pub struct CalendarResourceService<C: CalendarStore + ?Sized> {
-    pub cal_store: Arc<RwLock<C>>,
+    pub cal_store: Arc<C>,
     pub path: String,
     pub principal: String,
     pub calendar_id: String,
@@ -261,8 +260,6 @@ impl<C: CalendarStore + ?Sized> ResourceService for CalendarResourceService<C> {
         }
         let calendar = self
             .cal_store
-            .read()
-            .await
             .get_calendar(&self.principal, &self.calendar_id)
             .await
             .map_err(|_e| Error::NotFound)?;
@@ -275,8 +272,6 @@ impl<C: CalendarStore + ?Sized> ResourceService for CalendarResourceService<C> {
     ) -> Result<Vec<(String, Self::MemberType)>, Self::Error> {
         Ok(self
             .cal_store
-            .read()
-            .await
             .get_objects(&self.principal, &self.calendar_id)
             .await?
             .into_iter()
@@ -298,7 +293,7 @@ impl<C: CalendarStore + ?Sized> ResourceService for CalendarResourceService<C> {
         path_components: Self::PathComponents,
     ) -> Result<Self, Self::Error> {
         let cal_store = req
-            .app_data::<Data<RwLock<C>>>()
+            .app_data::<Data<C>>()
             .expect("no calendar store in app_data!")
             .clone()
             .into_inner();
@@ -313,8 +308,6 @@ impl<C: CalendarStore + ?Sized> ResourceService for CalendarResourceService<C> {
 
     async fn save_resource(&self, file: Self::Resource) -> Result<(), Self::Error> {
         self.cal_store
-            .write()
-            .await
             .update_calendar(
                 self.principal.to_owned(),
                 self.calendar_id.to_owned(),
@@ -326,8 +319,6 @@ impl<C: CalendarStore + ?Sized> ResourceService for CalendarResourceService<C> {
 
     async fn delete_resource(&self, use_trashbin: bool) -> Result<(), Self::Error> {
         self.cal_store
-            .write()
-            .await
             .delete_calendar(&self.principal, &self.calendar_id, use_trashbin)
             .await?;
         Ok(())

crates/caldav/src/calendar_object/methods.rs

@@ -7,7 +7,6 @@ use actix_web::HttpResponse;
 use rustical_store::auth::User;
 use rustical_store::model::CalendarObject;
 use rustical_store::CalendarStore;
-use tokio::sync::RwLock;
 use tracing::instrument;
 use tracing_actix_web::RootSpan;
 
@@ -16,7 +15,7 @@ use super::resource::CalendarObjectPathComponents;
 #[instrument(parent = root_span.id(), skip(store, root_span))]
 pub async fn get_event<C: CalendarStore + ?Sized>(
     path: Path<CalendarObjectPathComponents>,
-    store: Data<RwLock<C>>,
+    store: Data<C>,
     user: User,
     root_span: RootSpan,
 ) -> Result<HttpResponse, Error> {
@@ -30,16 +29,12 @@ pub async fn get_event<C: CalendarStore + ?Sized>(
         return Ok(HttpResponse::Unauthorized().body(""));
     }
 
-    let calendar = store.read().await.get_calendar(&principal, &cal_id).await?;
+    let calendar = store.get_calendar(&principal, &cal_id).await?;
     if user.id != calendar.principal {
         return Ok(HttpResponse::Unauthorized().body(""));
     }
 
-    let event = store
-        .read()
-        .await
-        .get_object(&principal, &cal_id, &object_id)
-        .await?;
+    let event = store.get_object(&principal, &cal_id, &object_id).await?;
 
     Ok(HttpResponse::Ok()
         .insert_header(("ETag", event.get_etag()))
@@ -50,7 +45,7 @@ pub async fn get_event<C: CalendarStore + ?Sized>(
 #[instrument(parent = root_span.id(), skip(store, req, root_span))]
 pub async fn put_event<C: CalendarStore + ?Sized>(
     path: Path<CalendarObjectPathComponents>,
-    store: Data<RwLock<C>>,
+    store: Data<C>,
     body: String,
     user: User,
     req: HttpRequest,
@@ -66,37 +61,16 @@ pub async fn put_event<C: CalendarStore + ?Sized>(
         return Ok(HttpResponse::Unauthorized().body(""));
     }
 
-    let calendar = store.read().await.get_calendar(&principal, &cal_id).await?;
-    if user.id != calendar.principal {
-        return Ok(HttpResponse::Unauthorized().body(""));
-    }
-
     // TODO: implement If-Match
     //
-    let mut store_write = store.write().await;
 
-    if Some(&HeaderValue::from_static("*")) == req.headers().get(header::IF_NONE_MATCH) {
-        // Only write if not existing
-        match store_write
-            .get_object(&principal, &cal_id, &object_id)
-            .await
-        {
-            Ok(_) => {
-                // Conflict
-                return Ok(HttpResponse::Conflict().body("Resource with this URI already exists"));
-            }
-            Err(rustical_store::Error::NotFound) => {
-                // Path unused, we can proceed
-            }
-            Err(err) => {
-                // Some unknown error :(
-                return Err(err.into());
-            }
-        }
-    }
+    let overwrite =
+        Some(&HeaderValue::from_static("*")) != req.headers().get(header::IF_NONE_MATCH);
 
     let object = CalendarObject::from_ics(object_id, body)?;
-    store_write.put_object(principal, cal_id, object).await?;
+    store
+        .put_object(principal, cal_id, object, overwrite)
+        .await?;
 
     Ok(HttpResponse::Created().body(""))
 }

crates/caldav/src/calendar_object/resource.rs

@@ -1,3 +1,4 @@
+use super::methods::{get_event, put_event};
 use crate::Error;
 use actix_web::{dev::ResourceMap, web::Data, HttpRequest};
 use async_trait::async_trait;
@@ -8,12 +9,9 @@ use rustical_store::CalendarStore;
 use serde::{Deserialize, Serialize};
 use std::sync::Arc;
 use strum::{EnumString, VariantNames};
-use tokio::sync::RwLock;
-
-use super::methods::{get_event, put_event};
 
 pub struct CalendarObjectResourceService<C: CalendarStore + ?Sized> {
-    pub cal_store: Arc<RwLock<C>>,
+    pub cal_store: Arc<C>,
     pub path: String,
     pub principal: String,
     pub cal_id: String,
@@ -121,7 +119,7 @@ impl<C: CalendarStore + ?Sized> ResourceService for CalendarObjectResourceServic
         } = path_components;
 
         let cal_store = req
-            .app_data::<Data<RwLock<C>>>()
+            .app_data::<Data<C>>()
             .expect("no calendar store in app_data!")
             .clone()
             .into_inner();
@@ -141,8 +139,6 @@ impl<C: CalendarStore + ?Sized> ResourceService for CalendarObjectResourceServic
         }
         let event = self
             .cal_store
-            .read()
-            .await
             .get_object(&self.principal, &self.cal_id, &self.object_id)
             .await?;
         Ok(event.into())
@@ -154,8 +150,6 @@ impl<C: CalendarStore + ?Sized> ResourceService for CalendarObjectResourceServic
 
     async fn delete_resource(&self, use_trashbin: bool) -> Result<(), Self::Error> {
         self.cal_store
-            .write()
-            .await
             .delete_object(&self.principal, &self.cal_id, &self.object_id, use_trashbin)
             .await?;
         Ok(())

crates/caldav/src/error.rs

@@ -29,7 +29,7 @@ impl actix_web::ResponseError for Error {
         match self {
             Error::StoreError(err) => match err {
                 rustical_store::Error::NotFound => StatusCode::NOT_FOUND,
-                rustical_store::Error::InvalidIcs(_) => StatusCode::BAD_REQUEST,
+                rustical_store::Error::InvalidData(_) => StatusCode::BAD_REQUEST,
                 _ => StatusCode::INTERNAL_SERVER_ERROR,
             },
             Error::DavError(err) => err.status_code(),

crates/caldav/src/lib.rs

@@ -11,7 +11,6 @@ use rustical_dav::resource::ResourceService;
 use rustical_store::auth::{AuthenticationMiddleware, AuthenticationProvider};
 use rustical_store::CalendarStore;
 use std::sync::Arc;
-use tokio::sync::RwLock;
 
 pub mod calendar;
 pub mod calendar_object;
@@ -28,7 +27,7 @@ pub fn configure_well_known(cfg: &mut web::ServiceConfig, caldav_root: String) {
 pub fn configure_dav<AP: AuthenticationProvider, C: CalendarStore + ?Sized>(
     cfg: &mut web::ServiceConfig,
     auth_provider: Arc<AP>,
-    store: Arc<RwLock<C>>,
+    store: Arc<C>,
 ) {
     cfg.service(
         web::scope("")

crates/caldav/src/principal/mod.rs

@@ -1,3 +1,4 @@
+use crate::calendar::resource::CalendarResource;
 use crate::Error;
 use actix_web::dev::ResourceMap;
 use actix_web::web::Data;
@@ -9,13 +10,10 @@ use rustical_store::CalendarStore;
 use serde::{Deserialize, Serialize};
 use std::sync::Arc;
 use strum::{EnumString, VariantNames};
-use tokio::sync::RwLock;
-
-use crate::calendar::resource::CalendarResource;
 
 pub struct PrincipalResourceService<C: CalendarStore + ?Sized> {
     principal: String,
-    cal_store: Arc<RwLock<C>>,
+    cal_store: Arc<C>,
 }
 
 #[derive(Clone)]
@@ -113,7 +111,7 @@ impl<C: CalendarStore + ?Sized> ResourceService for PrincipalResourceService<C>
         (principal,): Self::PathComponents,
     ) -> Result<Self, Self::Error> {
         let cal_store = req
-            .app_data::<Data<RwLock<C>>>()
+            .app_data::<Data<C>>()
             .expect("no calendar store in app_data!")
             .clone()
             .into_inner();
@@ -137,12 +135,7 @@ impl<C: CalendarStore + ?Sized> ResourceService for PrincipalResourceService<C>
         &self,
         rmap: &ResourceMap,
     ) -> Result<Vec<(String, Self::MemberType)>, Self::Error> {
-        let calendars = self
-            .cal_store
-            .read()
-            .await
-            .get_calendars(&self.principal)
-            .await?;
+        let calendars = self.cal_store.get_calendars(&self.principal).await?;
         Ok(calendars
             .into_iter()
             .map(|cal| {