Traits are closer to what actual calendars provide

Cargo.toml

@@ -7,8 +7,8 @@ edition = "2018"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [features]
-integration_tests = ["mock_version_tag"]
+integration_tests = ["local_calendar_mocks_remote_calendars"]
-mock_version_tag = []
+local_calendar_mocks_remote_calendars = []
 
 [dependencies]
 env_logger = "0.8"

src/cache.rs

@@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize};
 use async_trait::async_trait;
 
 use crate::traits::CalDavSource;
-use crate::traits::PartialCalendar;
+use crate::traits::BaseCalendar;
 use crate::traits::CompleteCalendar;
 use crate::calendar::cached_calendar::CachedCalendar;
 use crate::calendar::CalendarId;
@@ -143,21 +143,21 @@ impl Cache {
             let items_l = cal_l.get_items().await?;
             let items_r = cal_r.get_items().await?;
 
-                    if keys_are_the_same(&items_l, &items_r) == false {
+            if keys_are_the_same(&items_l, &items_r) == false {
                 log::debug!("Different keys for items");
-                        return Ok(false);
+                return Ok(false);
-}
+            }
-                    for (id_l, item_l) in items_l {
+            for (id_l, item_l) in items_l {
                 let item_r = match items_r.get(&id_l) {
                     Some(c) => c,
                     None => return Err("should not happen, we've just tested keys are the same".into()),
                 };
-                        if &item_l != item_r {
+                if &item_l != item_r {
                     log::debug!("Different items");
-                            return Ok(false);
+                    return Ok(false);
-                        }
-                    }
                 }
+            }
+        }
         Ok(true)
     }
 }
@@ -209,7 +209,6 @@ mod tests {
                             SupportedComponents::TODO);
         cache.add_calendar(Arc::new(Mutex::new(cal1)));
 
-
         cache.save_to_folder().unwrap();
 
         let retrieved_cache = Cache::from_folder(&cache_path).unwrap();

src/calendar/cached_calendar.rs

@@ -4,11 +4,11 @@ use std::error::Error;
 use serde::{Deserialize, Serialize};
 use async_trait::async_trait;
 
-use crate::{SyncStatus, traits::{PartialCalendar, CompleteCalendar}};
+use crate::SyncStatus;
+use crate::traits::{BaseCalendar, CompleteCalendar};
 use crate::calendar::{CalendarId, SupportedComponents};
 use crate::Item;
 use crate::item::ItemId;
-use crate::item::VersionTag;
 
 
 /// A calendar used by the [`cache`](crate::cache) module
@@ -29,8 +29,55 @@ impl CachedCalendar {
             items: HashMap::new(),
         }
     }
+}
 
-    pub async fn mark_for_deletion(&mut self, item_id: &ItemId) -> Result<(), Box<dyn Error>> {
+#[async_trait]
+impl BaseCalendar for CachedCalendar {
+    fn name(&self) -> &str {
+        &self.name
+    }
+
+    fn id(&self) -> &CalendarId {
+        &self.id
+    }
+
+    fn supported_components(&self) -> SupportedComponents {
+        self.supported_components
+    }
+
+    async fn add_item(&mut self, item: Item) -> Result<(), Box<dyn Error>> {
+        // TODO: here (and in the remote version, display an errror in case we overwrite something?)
+        self.items.insert(item.id().clone(), item);
+        Ok(())
+    }
+
+    async fn get_item_by_id<'a>(&'a self, id: &ItemId) -> Option<&'a Item> {
+        self.items.get(id)
+    }
+}
+
+#[async_trait]
+impl CompleteCalendar for CachedCalendar {
+    async fn get_item_ids(&self) -> Result<HashSet<ItemId>, Box<dyn Error>> {
+        eprintln!("Overridden implem");
+        Ok(self.items.iter()
+            .map(|(id, _)| id.clone())
+            .collect()
+        )
+    }
+
+    async fn get_items(&self) -> Result<HashMap<ItemId, &Item>, Box<dyn Error>> {
+        Ok(self.items.iter()
+            .map(|(id, item)| (id.clone(), item))
+            .collect()
+        )
+    }
+
+    async fn get_item_by_id_mut<'a>(&'a mut self, id: &ItemId) -> Option<&'a mut Item> {
+        self.items.get_mut(id)
+    }
+
+    async fn mark_for_deletion(&mut self, item_id: &ItemId) -> Result<(), Box<dyn Error>> {
         match self.items.get_mut(item_id) {
             None => Err("no item for this key".into()),
             Some(item) => {
@@ -56,47 +103,32 @@ impl CachedCalendar {
             }
         }
     }
+
+    async fn immediately_delete_item(&mut self, item_id: &ItemId) -> Result<(), Box<dyn Error>> {
+        match self.items.remove(item_id) {
+            None => Err(format!("Item {} is absent from this calendar", item_id).into()),
+            Some(_) => Ok(())
+        }
+    }
 }
 
+
+
+// This class can be used to mock a remote calendar for integration tests
+
+#[cfg(feature = "local_calendar_mocks_remote_calendars")]
+use crate::{item::VersionTag,
+            traits::DavCalendar};
+
+#[cfg(feature = "local_calendar_mocks_remote_calendars")]
 #[async_trait]
-impl PartialCalendar for CachedCalendar {
+impl DavCalendar for CachedCalendar {
-    fn name(&self) -> &str {
-        &self.name
-    }
-
-    fn id(&self) -> &CalendarId {
-        &self.id
-    }
-
-    fn supported_components(&self) -> SupportedComponents {
-        self.supported_components
-    }
-
-    async fn add_item(&mut self, item: Item) -> Result<(), Box<dyn Error>> {
-        self.items.insert(item.id().clone(), item);
-        Ok(())
-    }
-
-    async fn delete_item(&mut self, item_id: &ItemId) -> Result<(), Box<dyn Error>> {
-        if let None = self.items.remove(item_id) {
-            return Err("This key does not exist.".into());
-        }
-        Ok(())
-    }
-
-    #[cfg(not(feature = "mock_version_tag"))]
-    #[allow(unreachable_code)]
-    async fn get_item_version_tags(&self) -> Result<HashMap<ItemId, VersionTag>, Box<dyn Error>> {
-        panic!("This function only makes sense in remote calendars and in mocked calendars");
-        Err("This function only makes sense in remote calendars and in mocked calendars".into())
-    }
-    #[cfg(feature = "mock_version_tag")]
     async fn get_item_version_tags(&self) -> Result<HashMap<ItemId, VersionTag>, Box<dyn Error>> {
         use crate::item::SyncStatus;
 
         let mut result = HashMap::new();
 
-        for (id, item) in &self.items {
+        for (id, item) in self.items.iter() {
             let vt = match item.sync_status() {
                 SyncStatus::Synced(vt) => vt.clone(),
                 _ => {
@@ -109,32 +141,7 @@ impl PartialCalendar for CachedCalendar {
         Ok(result)
     }
 
-    // This reimplements the trait method to avoid resorting to `get_item_version_tags`
+    async fn delete_item(&mut self, item_id: &ItemId) -> Result<(), Box<dyn Error>> {
-    // (this is thus slighlty faster, but also avoids an unnecessary iteration over SyncStatus that might panic for some mocked values if feature `mock_version_tag` is set)
+        self.immediately_delete_item(item_id).await
-    async fn get_item_ids(&self) -> Result<HashSet<ItemId>, Box<dyn Error>> {
-        Ok(self.items.iter()
-            .map(|(id, _)| id.clone())
-            .collect()
-        )
-    }
-
-
-    async fn get_item_by_id_mut<'a>(&'a mut self, id: &ItemId) -> Option<&'a mut Item> {
-        self.items.get_mut(id)
-    }
-
-    async fn get_item_by_id<'a>(&'a self, id: &ItemId) -> Option<&'a Item> {
-        self.items.get(id)
-    }
-}
-
-#[async_trait]
-impl CompleteCalendar for CachedCalendar {
-    /// Returns the list of items that this calendar contains
-    async fn get_items(&self) -> Result<HashMap<ItemId, &Item>, Box<dyn Error>> {
-        Ok(self.items.iter()
-            .map(|(id, item)| (id.clone(), item))
-            .collect()
-        )
     }
 }

src/calendar/remote_calendar.rs

@@ -1,9 +1,10 @@
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 use std::error::Error;
 
 use async_trait::async_trait;
 
-use crate::traits::PartialCalendar;
+use crate::traits::BaseCalendar;
+use crate::traits::DavCalendar;
 use crate::calendar::SupportedComponents;
 use crate::calendar::CalendarId;
 use crate::item::Item;
@@ -44,63 +45,34 @@ impl RemoteCalendar {
 }
 
 #[async_trait]
-impl PartialCalendar for RemoteCalendar {
+impl BaseCalendar for RemoteCalendar {
     fn name(&self) -> &str { &self.name }
     fn id(&self) -> &CalendarId { &self.resource.url() }
     fn supported_components(&self) -> crate::calendar::SupportedComponents {
         self.supported_components
     }
 
-    /// Get the IDs of all current items in this calendar
+    /// Add an item into this calendar
-    async fn get_item_ids(&self) -> Result<HashSet<ItemId>, Box<dyn Error>> {
+    async fn add_item(&mut self, _item: Item) -> Result<(), Box<dyn Error>> {
-        let responses = crate::client::sub_request_and_extract_elems(&self.resource, "REPORT", TASKS_BODY.to_string(), "response").await?;
+        Err("Not implemented".into())
-
-        let mut item_ids = HashSet::new();
-        for response in responses {
-            let item_url = crate::utils::find_elem(&response, "href")
-                .map(|elem| self.resource.combine(&elem.text()));
-
-            match item_url {
-                None => {
-                    log::warn!("Unable to extract HREF");
-                    continue;
-                },
-                Some(resource) => {
-                    item_ids.insert(ItemId::from(&resource));
-                },
-            };
-        }
-
-        Ok(item_ids)
-    }
-
-    async fn get_item_version_tags(&self) -> Result<HashMap<ItemId, VersionTag>, Box<dyn Error>> {
-        log::error!("Not implemented");
-        Ok(HashMap::new())
-    }
-
-    /// Returns a particular item
-    async fn get_item_by_id_mut<'a>(&'a mut self, _id: &ItemId) -> Option<&'a mut Item> {
-        log::error!("Not implemented");
-        None
     }
 
     async fn get_item_by_id<'a>(&'a self, id: &ItemId) -> Option<&'a Item> {
         log::error!("Not implemented");
         None
     }
+}
 
-
+#[async_trait]
-    /// Add an item into this calendar
+impl DavCalendar for RemoteCalendar {
-    async fn add_item(&mut self, _item: Item) -> Result<(), Box<dyn Error>> {
+    async fn get_item_version_tags(&self) -> Result<HashMap<ItemId, VersionTag>, Box<dyn Error>> {
-        Err("Not implemented".into())
+        log::error!("Not implemented");
+        Ok(HashMap::new())
     }
 
-    /// Remove an item from this calendar
     async fn delete_item(&mut self, _item_id: &ItemId) -> Result<(), Box<dyn Error>> {
         log::error!("Not implemented");
         Ok(())
     }
-
 }
 

src/client.rs

@@ -15,7 +15,7 @@ use crate::utils::{find_elem, find_elems};
 use crate::calendar::remote_calendar::RemoteCalendar;
 use crate::calendar::CalendarId;
 use crate::traits::CalDavSource;
-use crate::traits::PartialCalendar;
+use crate::traits::BaseCalendar;
 
 
 static DAVCLIENT_BODY: &str = r#"

src/item.rs

@@ -132,7 +132,7 @@ impl From<String> for VersionTag {
 
 impl VersionTag {
     /// Generate a random VesionTag
-    #[cfg(feature = "mock_version_tag")]
+    #[cfg(feature = "local_calendar_mocks_remote_calendars")]
     pub fn random() -> Self {
         let random = uuid::Uuid::new_v4().to_hyphenated().to_string();
         Self { tag: random }
@@ -146,7 +146,8 @@ impl VersionTag {
 pub enum SyncStatus {
     /// This item has ben locally created, and never synced yet
     NotSynced,
-    /// At the time this item has ben synced, it has a given version tag, and has not been locally modified since then
+    /// At the time this item has ben synced, it has a given version tag, and has not been locally modified since then.
+    /// Note: in integration tests, in case we are mocking a remote calendar by a local calendar, this is the only valid variant (remote calendars make no distinction between all these variants)
     Synced(VersionTag),
     /// This item has been synced when it had a given version tag, and has been locally modified since then.
     LocallyModified(VersionTag),
@@ -155,7 +156,7 @@ pub enum SyncStatus {
 }
 impl SyncStatus {
     /// Generate a random SyncStatus::Synced
-    #[cfg(feature = "mock_version_tag")]
+    #[cfg(feature = "local_calendar_mocks_remote_calendars")]
     pub fn random_synced() -> Self {
         Self::Synced(VersionTag::random())
     }

src/provider.rs

@@ -4,8 +4,8 @@ use std::error::Error;
 use std::collections::HashSet;
 use std::marker::PhantomData;
 
-use crate::traits::{CalDavSource, CompleteCalendar};
+use crate::traits::{CalDavSource, DavCalendar};
-use crate::traits::PartialCalendar;
+use crate::traits::CompleteCalendar;
 use crate::item::SyncStatus;
 
 /// A data source that combines two `CalDavSources` (usually a server and a local cache), which is able to sync both sources.
@@ -14,7 +14,7 @@ where
     L: CalDavSource<T>,
     T: CompleteCalendar + Sync + Send,
     R: CalDavSource<U>,
-    U: PartialCalendar + Sync + Send,
+    U: DavCalendar + Sync + Send,
 {
     /// The remote source (usually a server)
     remote: R,
@@ -30,7 +30,7 @@ where
     L: CalDavSource<T>,
     T: CompleteCalendar + Sync + Send,
     R: CalDavSource<U>,
-    U: PartialCalendar + Sync + Send,
+    U: DavCalendar + Sync + Send,
 {
     /// Create a provider.
     ///
@@ -155,7 +155,7 @@ where
             }
 
             for id_del in remote_del {
-                if let Err(err) = cal_local.delete_item(&id_del).await {
+                if let Err(err) = cal_local.immediately_delete_item(&id_del).await {
                     log::warn!("Unable to delete local item {}: {}", id_del, err);
                 }
             }

src/task.rs

@@ -37,18 +37,44 @@ impl Task {
         self.sync_status = new_status;
     }
 
-    fn update_last_modified(&mut self) {
+    fn update_sync_status(&mut self) {
+        match &self.sync_status {
+            SyncStatus::NotSynced => return,
+            SyncStatus::LocallyModified(_) => return,
+            SyncStatus::Synced(prev_vt) => {
+                self.sync_status = SyncStatus::LocallyModified(prev_vt.clone());
+            }
+            SyncStatus::LocallyDeleted(_) => {
+                log::warn!("Trying to update an item that has previously been deleted. These changes will probably be ignored at next sync.");
+                return;
+            },
+        }
     }
 
     /// Rename a task.
     /// This updates its "last modified" field
     pub fn set_name(&mut self, new_name: String) {
+        self.update_sync_status();
+        self.name = new_name;
+    }
+    #[cfg(feature = "local_calendar_mocks_remote_calendars")]
+    /// Rename a task, but forces a "master" SyncStatus, just like CalDAV servers are always "masters"
+    pub fn mock_remote_calendar_set_name(&mut self, new_name: String) {
+        self.sync_status = SyncStatus::random_synced();
         self.name = new_name;
     }
 
+    /// Set the completion status
     pub fn set_completed(&mut self, new_value: bool) {
         // TODO: either require a reference to the DataSource, so that it is aware
         //       or change a flag here, and the DataSource will be able to check the flags of all its content (but then the Calendar should only give a reference/Arc, not a clone)
+        self.update_sync_status();
+        self.completed = new_value;
+    }
+    #[cfg(feature = "local_calendar_mocks_remote_calendars")]
+    /// Set the completion status, but forces a "master" SyncStatus, just like CalDAV servers are always "masters"
+    pub fn mock_remote_calendar_set_completed(&mut self, new_value: bool) {
+        self.sync_status = SyncStatus::random_synced();
         self.completed = new_value;
     }
 }

src/traits.rs

@@ -10,7 +10,7 @@ use crate::item::VersionTag;
 use crate::calendar::CalendarId;
 
 #[async_trait]
-pub trait CalDavSource<T: PartialCalendar> {
+pub trait CalDavSource<T: BaseCalendar> {
     /// Returns the current calendars that this source contains
     /// This function may trigger an update (that can be a long process, or that can even fail, e.g. in case of a remote server)
     async fn get_calendars(&self) -> Result<HashMap<CalendarId, Arc<Mutex<T>>>, Box<dyn Error>>;
@@ -18,11 +18,9 @@ pub trait CalDavSource<T: PartialCalendar> {
     async fn get_calendar(&self, id: &CalendarId) -> Option<Arc<Mutex<T>>>;
 }
 
-/// A calendar we have a partial knowledge of.
+/// This trait contains functions that are common to all calendars
-///
-/// Usually, this is a calendar from a remote source, that is synced to a CompleteCalendar
 #[async_trait]
-pub trait PartialCalendar {
+pub trait BaseCalendar {
     /// Returns the calendar name
     fn name(&self) -> &str;
 
@@ -32,20 +30,11 @@ pub trait PartialCalendar {
     /// Returns the supported kinds of components for this calendar
     fn supported_components(&self) -> crate::calendar::SupportedComponents;
 
-    /// Get the IDs and the version tags of every item in this calendar
-    async fn get_item_version_tags(&self) -> Result<HashMap<ItemId, VersionTag>, Box<dyn Error>>;
-
-    /// Returns a particular item
-    async fn get_item_by_id_mut<'a>(&'a mut self, id: &ItemId) -> Option<&'a mut Item>;
-
-    /// Returns a particular item
-    async fn get_item_by_id<'a>(&'a self, id: &ItemId) -> Option<&'a Item>;
-
     /// Add an item into this calendar
     async fn add_item(&mut self, item: Item) -> Result<(), Box<dyn Error>>;
 
-    /// Remove an item from this calendar
+    /// Returns a particular item
-    async fn delete_item(&mut self, item_id: &ItemId) -> Result<(), Box<dyn Error>>;
+    async fn get_item_by_id<'a>(&'a self, id: &ItemId) -> Option<&'a Item>;
 
 
     /// Returns whether this calDAV calendar supports to-do items
@@ -57,6 +46,17 @@ pub trait PartialCalendar {
     fn supports_events(&self) -> bool {
         self.supported_components().contains(crate::calendar::SupportedComponents::EVENT)
     }
+}
+
+
+/// Functions availabe for calendars that are backed by a CalDAV server
+#[async_trait]
+pub trait DavCalendar : BaseCalendar {
+    /// Get the IDs and the version tags of every item in this calendar
+    async fn get_item_version_tags(&self) -> Result<HashMap<ItemId, VersionTag>, Box<dyn Error>>;
+
+    /// Delete an item
+    async fn delete_item(&mut self, item_id: &ItemId) -> Result<(), Box<dyn Error>>;
 
     /// Get the IDs of all current items in this calendar
     async fn get_item_ids(&self) -> Result<HashSet<ItemId>, Box<dyn Error>> {
@@ -65,20 +65,28 @@ pub trait PartialCalendar {
             .map(|(id, _tag)| id.clone())
             .collect())
     }
-
-    /// Finds the IDs of the items that are missing compared to a reference set
-    async fn find_deletions_from(&self, reference_set: HashSet<ItemId>) -> Result<HashSet<ItemId>, Box<dyn Error>> {
-        let current_items = self.get_item_ids().await?;
-        Ok(reference_set.difference(&current_items).map(|id| id.clone()).collect())
-    }
 }
 
-/// A calendar we always know everything about.
+
+/// Functions availabe for calendars we have full knowledge of
 ///
-/// Usually, this is a calendar fully stored on a local disk
+/// Usually, these are local calendars fully backed by a local folder
 #[async_trait]
-pub trait CompleteCalendar : PartialCalendar {
+pub trait CompleteCalendar : BaseCalendar {
-    /// Returns the list of items that this calendar contains
+    /// Get the IDs of all current items in this calendar
-    async fn get_items(&self) -> Result<HashMap<ItemId, &Item>, Box<dyn Error>>;
+    async fn get_item_ids(&self) -> Result<HashSet<ItemId>, Box<dyn Error>>;
-}
 
+    /// Returns all items that this calendar contains
+    async fn get_items(&self) -> Result<HashMap<ItemId, &Item>, Box<dyn Error>>;
+
+    /// Returns a particular item
+    async fn get_item_by_id_mut<'a>(&'a mut self, id: &ItemId) -> Option<&'a mut Item>;
+
+    /// Mark an item for deletion.
+    /// This is required so that the upcoming sync will know it should also also delete this task from the server
+    /// (and then call [`immediately_delete_item`] once it has been successfully deleted on the server)
+    async fn mark_for_deletion(&mut self, item_id: &ItemId) -> Result<(), Box<dyn Error>>;
+
+    /// Immediately remove an item. See [`mark_for_deletion`]
+    async fn immediately_delete_item(&mut self, item_id: &ItemId) -> Result<(), Box<dyn Error>>;
+}

tests/caldav_client.rs

@@ -7,7 +7,8 @@ use minidom::Element;
 use url::Url;
 
 use my_tasks::client::Client;
-use my_tasks::traits::PartialCalendar;
+use my_tasks::traits::BaseCalendar;
+use my_tasks::traits::DavCalendar;
 use my_tasks::traits::CalDavSource;
 
 use my_tasks::settings::URL;

tests/sync.rs

@@ -6,7 +6,9 @@ use std::sync::{Arc, Mutex};
 use url::Url;
 
 use my_tasks::traits::CalDavSource;
-use my_tasks::traits::PartialCalendar;
+use my_tasks::traits::BaseCalendar;
+use my_tasks::traits::CompleteCalendar;
+use my_tasks::traits::DavCalendar;
 use my_tasks::cache::Cache;
 use my_tasks::Item;
 use my_tasks::ItemId;
@@ -109,21 +111,21 @@ async fn populate_test_provider() -> Provider<Cache, CachedCalendar, Cache, Cach
     cal_server.delete_item(&task_b_id).await.unwrap();
 
     cal_server.get_item_by_id_mut(&task_e_id).await.unwrap().unwrap_task_mut()
-        .set_name("E has been remotely renamed".into());
+        .mock_remote_calendar_set_name("E has been remotely renamed".into());
 
     cal_server.get_item_by_id_mut(&task_f_id).await.unwrap().unwrap_task_mut()
-        .set_name("F renamed in the server".into());
+        .mock_remote_calendar_set_name("F renamed in the server".into());
 
     cal_server.get_item_by_id_mut(&task_g_id).await.unwrap().unwrap_task_mut()
-        .set_completed(true);
+        .mock_remote_calendar_set_completed(true);
 
     cal_server.get_item_by_id_mut(&task_i_id).await.unwrap().unwrap_task_mut()
-        .set_name("I renamed in the server".into());
+        .mock_remote_calendar_set_name("I renamed in the server".into());
 
     cal_server.delete_item(&task_j_id).await.unwrap();
 
     cal_server.get_item_by_id_mut(&task_k_id).await.unwrap().unwrap_task_mut()
-        .set_completed(true);
+        .mock_remote_calendar_set_completed(true);
 
     cal_server.delete_item(&task_l_id).await.unwrap();