The Big Type Overhaul

src/cache.rs

@@ -4,11 +4,11 @@ use std::path::PathBuf;
 use std::path::Path;
 use std::error::Error;
 use std::collections::HashMap;
+use std::collections::HashSet;
 use std::hash::Hash;
 
 use serde::{Deserialize, Serialize};
 use async_trait::async_trait;
-use url::Url;
 use chrono::{DateTime, Utc};
 
 use crate::traits::CalDavSource;
@@ -16,6 +16,7 @@ use crate::traits::SyncSlave;
 use crate::traits::PartialCalendar;
 use crate::traits::CompleteCalendar;
 use crate::calendar::cached_calendar::CachedCalendar;
+use crate::calendar::CalendarId;
 
 
 /// A CalDAV source that stores its item in a local file
@@ -27,7 +28,7 @@ pub struct Cache {
 
 #[derive(Default, Debug, PartialEq, Serialize, Deserialize)]
 struct CachedData {
-    calendars: Vec<CachedCalendar>,
+    calendars: HashMap<CalendarId, CachedCalendar>,
     last_sync: Option<DateTime<Utc>>,
 }
 
@@ -81,7 +82,7 @@ impl Cache {
 
 
     pub fn add_calendar(&mut self, calendar: CachedCalendar) {
-        self.data.calendars.push(calendar);
+        self.data.calendars.insert(calendar.id().clone(), calendar);
     }
 
     /// Compares two Caches to check they have the same current content
@@ -91,9 +92,16 @@ impl Cache {
         let calendars_l = self.get_calendars().await?;
         let calendars_r = other.get_calendars().await?;
 
-        for cal_l in calendars_l {
+        if keys_are_the_same(&calendars_l, &calendars_r) == false {
-            for cal_r in calendars_r {
+            return Ok(false);
-                if cal_l.url() == cal_r.url() {
+        }
+
+        for (id, cal_l) in calendars_l {
+            let cal_r = match calendars_r.get(id) {
+                Some(c) => c,
+                None => return Err("should not happen, we've just tested keys are the same".into()),
+            };
+
                     let items_l = cal_l.get_items();
                     let items_r = cal_r.get_items();
 
@@ -101,59 +109,52 @@ impl Cache {
                         return Ok(false);
 }
                     for (id_l, item_l) in items_l {
-                        let item_r = items_r.get(&id_l).unwrap();
+                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()),
+                };
                         //println!("  items {} {}", item_r.name(), item_l.name());
                         if &item_l != item_r {
                             return Ok(false);
                         }
                     }
-
-                    break;
                 }
-            }
-        }
-        // TODO: also check there is no missing calendar in one side or the other
-        // TODO: also check there is no missing task in either side of each calendar
         Ok(true)
     }
 }
 
 fn keys_are_the_same<T, U, V>(left: &HashMap<T, U>, right: &HashMap<T, V>) -> bool
 where
-    T: Hash + Eq,
+    T: Hash + Eq + Clone,
 {
-    left.len() == right.len()
+    if left.len() != right.len() {
-    && left.keys().all(|k| right.contains_key(k))
+        return false;
+    }
+
+    let keys_l: HashSet<T> = left.keys().cloned().collect();
+    let keys_r: HashSet<T> = right.keys().cloned().collect();
+    keys_l == keys_r
 }
 
 #[async_trait]
 impl CalDavSource<CachedCalendar> for Cache {
-    async fn get_calendars(&self) -> Result<&Vec<CachedCalendar>, Box<dyn Error>> {
+    async fn get_calendars(&self) -> Result<&HashMap<CalendarId, CachedCalendar>, Box<dyn Error>> {
         Ok(&self.data.calendars)
     }
 
-    async fn get_calendars_mut(&mut self) -> Result<Vec<&mut CachedCalendar>, Box<dyn Error>> {
+    async fn get_calendars_mut(&mut self) -> Result<HashMap<CalendarId, &mut CachedCalendar>, Box<dyn Error>> {
-        Ok(
+        let mut hm = HashMap::new();
-            self.data.calendars.iter_mut()
+        for (id, val) in self.data.calendars.iter_mut() {
-            .collect()
+            hm.insert(id.clone(), val);
-        )
     }
+        Ok(hm)
+            }
 
-    async fn get_calendar(&self, url: Url) -> Option<&CachedCalendar> {
+    async fn get_calendar(&self, id: CalendarId) -> Option<&CachedCalendar> {
-        for cal in &self.data.calendars {
+        self.data.calendars.get(&id)
-            if cal.url() == &url {
-                return Some(cal);
-            }
         }
-        return None;
+    async fn get_calendar_mut(&mut self, id: CalendarId) -> Option<&mut CachedCalendar> {
-    }
+        self.data.calendars.get_mut(&id)
-    async fn get_calendar_mut(&mut self, url: Url) -> Option<&mut CachedCalendar> {
-        for cal in &mut self.data.calendars {
-            if cal.url() == &url {
-                return Some(cal);
-            }
-        }
-        return None;
     }
 }
 

src/calendar/cached_calendar.rs

@@ -1,12 +1,12 @@
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::collections::BTreeMap;
+use std::error::Error;
 
-use url::Url;
 use serde::{Deserialize, Serialize};
 use chrono::{DateTime, Utc};
 
 use crate::traits::{PartialCalendar, CompleteCalendar};
-use crate::calendar::{SupportedComponents, SearchFilter};
+use crate::calendar::{CalendarId, SupportedComponents, SearchFilter};
 use crate::Item;
 use crate::item::ItemId;
 
@@ -15,19 +15,19 @@ use crate::item::ItemId;
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
 pub struct CachedCalendar {
     name: String,
-    url: Url,
+    id: CalendarId,
     supported_components: SupportedComponents,
 
-    items: Vec<Item>,
+    items: HashMap<ItemId, Item>,
     deleted_items: BTreeMap<DateTime<Utc>, ItemId>,
 }
 
 impl CachedCalendar {
     /// Create a new calendar
-    pub fn new(name: String, url: Url, supported_components: SupportedComponents) -> Self {
+    pub fn new(name: String, id: CalendarId, supported_components: SupportedComponents) -> Self {
         Self {
-            name, url, supported_components,
+            name, id, supported_components,
-            items: Vec::new(),
+            items: HashMap::new(),
             deleted_items: BTreeMap::new(),
         }
     }
@@ -47,8 +47,8 @@ impl PartialCalendar for CachedCalendar {
         &self.name
     }
 
-    fn url(&self) -> &Url {
+    fn id(&self) -> &CalendarId {
-        &self.url
+        &self.id
     }
 
     fn supported_components(&self) -> SupportedComponents {
@@ -56,12 +56,15 @@ impl PartialCalendar for CachedCalendar {
     }
 
     fn add_item(&mut self, item: Item) {
-        self.items.push(item);
+        self.items.insert(item.id().clone(), item);
     }
 
-    fn delete_item(&mut self, item_id: &ItemId) {
+    fn delete_item(&mut self, item_id: &ItemId) -> Result<(), Box<dyn Error>> {
-        self.items.retain(|i| i.id() != item_id);
+        if let None = self.items.remove(item_id) {
+            return Err("This key does not exist.".into());
+        }
         self.deleted_items.insert(Utc::now(), item_id.clone());
+        Ok(())
     }
 
     fn get_items_modified_since(&self, since: Option<DateTime<Utc>>, filter: Option<SearchFilter>) -> HashMap<ItemId, &Item> {
@@ -69,7 +72,7 @@ impl PartialCalendar for CachedCalendar {
 
         let mut map = HashMap::new();
 
-        for item in &self.items {
+        for (_id, item) in &self.items {
             match since {
                 None => (),
                 Some(since) => if item.last_modified() < since {
@@ -92,28 +95,21 @@ impl PartialCalendar for CachedCalendar {
         map
     }
 
-    fn get_item_ids(&mut self) -> Vec<ItemId> {
+    fn get_item_ids(&mut self) -> HashSet<ItemId> {
-        self.items.iter()
+        self.items.keys().cloned().collect()
-            .map(|item| item.id().clone())
-            .collect()
     }
 
     fn get_item_by_id_mut(&mut self, id: &ItemId) -> Option<&mut Item> {
-        for item in &mut self.items {
+        self.items.get_mut(id)
-            if item.id() == id {
-                return Some(item);
-            }
-        }
-        return None;
     }
 }
 
 impl CompleteCalendar for CachedCalendar {
     /// Returns the items that have been deleted after `since`
-    fn get_items_deleted_since(&self, since: DateTime<Utc>) -> Vec<ItemId> {
+    fn get_items_deleted_since(&self, since: DateTime<Utc>) -> HashSet<ItemId> {
         self.deleted_items.range(since..)
-        .map(|(_key, value)| value.clone())
+            .map(|(_key, id)| id.clone())
-        .collect()
+            .collect()
     }
 
     /// Returns the list of items that this calendar contains

src/calendar/mod.rs

@@ -61,3 +61,6 @@ impl Default for SearchFilter {
         SearchFilter::All
     }
 }
+
+
+pub type CalendarId = url::Url;

src/client.rs

@@ -2,6 +2,7 @@
 
 use std::error::Error;
 use std::convert::TryFrom;
+use std::collections::HashMap;
 
 use reqwest::Method;
 use reqwest::header::CONTENT_TYPE;
@@ -10,6 +11,7 @@ use url::Url;
 
 use crate::utils::{find_elem, find_elems};
 use crate::calendar::cached_calendar::CachedCalendar;
+use crate::calendar::CalendarId;
 use crate::traits::PartialCalendar;
 
 
@@ -71,7 +73,7 @@ pub struct Client {
 
     principal: Option<Url>,
     calendar_home_set: Option<Url>,
-    calendars: Option<Vec<CachedCalendar>>,
+    calendars: Option<HashMap<CalendarId, CachedCalendar>>,
 }
 
 impl Client {
@@ -150,9 +152,9 @@ impl Client {
     }
 
     /// Return the list of calendars, or fetch from server if not known yet
-    pub async fn get_calendars(&mut self) -> Result<Vec<CachedCalendar>, Box<dyn Error>> {
+    pub async fn get_calendars(&mut self) -> Result<HashMap<CalendarId, CachedCalendar>, Box<dyn Error>> {
         if let Some(c) = &self.calendars {
-            return Ok(c.to_vec());
+            return Ok(c.clone());
         }
         let cal_home_set = self.get_cal_home_set().await?;
 
@@ -160,7 +162,7 @@ impl Client {
 
         let root: Element = text.parse().unwrap();
         let reps = find_elems(&root, "response");
-        let mut calendars = Vec::new();
+        let mut calendars = HashMap::new();
         for rep in reps {
             let display_name = find_elem(rep, "displayname").map(|e| e.text()).unwrap_or("<no name>".to_string());
             log::debug!("Considering calendar {}", display_name);
@@ -210,14 +212,14 @@ impl Client {
             };
             let this_calendar = CachedCalendar::new(display_name, this_calendar_url, supported_components);
             log::info!("Found calendar {}", this_calendar.name());
-            calendars.push(this_calendar);
+            calendars.insert(this_calendar.id().clone(), this_calendar);
         }
 
         self.calendars = Some(calendars.clone());
         Ok(calendars)
     }
 
-    pub async fn get_tasks(&mut self, calendar: &Url) -> Result<(), Box<dyn Error>> {
+    pub async fn get_tasks(&mut self, calendar: &CalendarId) -> Result<(), Box<dyn Error>> {
         let method = Method::from_bytes(b"REPORT")
             .expect("cannot create REPORT method.");
 

src/provider.rs

@@ -1,6 +1,8 @@
 //! This modules abstracts data sources and merges them in a single virtual one
 
-use std::{error::Error, marker::PhantomData};
+use std::error::Error;
+use std::collections::{HashSet, HashMap};
+use std::marker::PhantomData;
 
 use chrono::{DateTime, Utc};
 
@@ -63,8 +65,8 @@ where
         log::info!("Starting a sync. Last sync was at {:?}", last_sync);
         let cals_server = self.server.get_calendars_mut().await?;
 
-        for cal_server in cals_server {
+        for (id, cal_server) in cals_server {
-            let cal_local = match self.local.get_calendar_mut(cal_server.url().clone()).await {
+            let cal_local = match self.local.get_calendar_mut(id).await {
                 None => {
                     log::error!("TODO: implement here");
                     continue;
@@ -72,21 +74,16 @@ where
                 Some(cal) => cal,
             };
 
-            let server_del = match last_sync {
+            // Pull remote changes from the server
-                Some(_date) => cal_server.find_deletions(cal_local.get_item_ids()),
+            let mut tasks_id_to_remove_from_local = match last_sync {
-                None => Vec::new(),
-            };
-            let local_del = match last_sync {
-                Some(date) => cal_local.get_items_deleted_since(date),
                 None => Vec::new(),
+                Some(_date) => cal_server.find_deletions_from(cal_local.get_item_ids())
+                    .iter()
+                    .map(|id| id.clone())
+                    .collect()
             };
 
-            // Pull remote changes from the server
             let mut tasks_to_add_to_local = Vec::new();
-            let mut tasks_id_to_remove_from_local = Vec::new();
-            for deleted_id in server_del {
-                tasks_id_to_remove_from_local.push(deleted_id);
-            }
             let server_mod = cal_server.get_items_modified_since(last_sync, None);
             for (new_id, new_item) in &server_mod {
                 if server_mod.contains_key(new_id) {
@@ -101,9 +98,10 @@ where
 
 
             // Push local changes to the server
-            let local_mod = cal_local.get_items_modified_since(last_sync, None);
+            let local_del = match last_sync {
-
+                Some(date) => cal_local.get_items_deleted_since(date),
-            let mut tasks_to_add_to_server = Vec::new();
+                None => HashSet::new(),
+            };
             let mut tasks_id_to_remove_from_server = Vec::new();
             for deleted_id in local_del {
                 if server_mod.contains_key(&deleted_id) {
@@ -112,6 +110,9 @@ where
                 }
                 tasks_id_to_remove_from_server.push(deleted_id);
             }
+
+            let local_mod = cal_local.get_items_modified_since(last_sync, None);
+            let mut tasks_to_add_to_server = Vec::new();
             for (new_id, new_item) in &local_mod {
                 if server_mod.contains_key(new_id) {
                     log::warn!("Conflict for task {} ({}). Using the server version.", new_item.name(), new_id);

src/traits.rs

@@ -1,31 +1,31 @@
 use std::error::Error;
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 
 use async_trait::async_trait;
-use url::Url;
 use chrono::{DateTime, Utc};
 
 use crate::item::Item;
 use crate::item::ItemId;
+use crate::calendar::CalendarId;
 
 #[async_trait]
 pub trait CalDavSource<T: PartialCalendar> {
     /// 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<&Vec<T>, Box<dyn Error>>;
+    async fn get_calendars(&self) -> Result<&HashMap<CalendarId, T>, Box<dyn Error>>;
     /// 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_mut(&mut self) -> Result<Vec<&mut T>, Box<dyn Error>>;
+    async fn get_calendars_mut(&mut self) -> Result<HashMap<CalendarId, &mut T>, Box<dyn Error>>;
 
     //
     //
     // TODO: find a better search key (do calendars have a unique ID?)
     // TODO: search key should be a reference
     //
-    /// Returns the calendar matching the URL
+    /// Returns the calendar matching the ID
-    async fn get_calendar(&self, url: Url) -> Option<&T>;
+    async fn get_calendar(&self, id: CalendarId) -> Option<&T>;
-    /// Returns the calendar matching the URL
+    /// Returns the calendar matching the ID
-    async fn get_calendar_mut(&mut self, url: Url) -> Option<&mut T>;
+    async fn get_calendar_mut(&mut self, id: CalendarId) -> Option<&mut T>;
 
 }
 
@@ -44,8 +44,8 @@ pub trait PartialCalendar {
     /// Returns the calendar name
     fn name(&self) -> &str;
 
-    /// Returns the calendar URL
+    /// Returns the calendar unique ID
-    fn url(&self) -> &Url;
+    fn id(&self) -> &CalendarId;
 
     /// Returns the supported kinds of components for this calendar
     fn supported_components(&self) -> crate::calendar::SupportedComponents;
@@ -55,7 +55,7 @@ pub trait PartialCalendar {
         -> HashMap<ItemId, &Item>;
 
     /// Get the IDs of all current items in this calendar
-    fn get_item_ids(&mut self) -> Vec<ItemId>;
+    fn get_item_ids(&mut self) -> HashSet<ItemId>;
 
     /// Returns a particular item
     fn get_item_by_id_mut(&mut self, id: &ItemId) -> Option<&mut Item>;
@@ -64,7 +64,7 @@ pub trait PartialCalendar {
     fn add_item(&mut self, item: Item);
 
     /// Remove an item from this calendar
-    fn delete_item(&mut self, item_id: &ItemId);
+    fn delete_item(&mut self, item_id: &ItemId) -> Result<(), Box<dyn Error>>;
 
 
     /// Returns whether this calDAV calendar supports to-do items
@@ -78,16 +78,9 @@ pub trait PartialCalendar {
     }
 
     /// Finds the IDs of the items that are missing compared to a reference set
-    fn find_deletions(&mut self, reference_set: Vec<ItemId>) -> Vec<ItemId> {
+    fn find_deletions_from(&mut self, reference_set: HashSet<ItemId>) -> HashSet<ItemId> {
-        let mut deletions = Vec::new();
-
         let current_items = self.get_item_ids();
-        for original_item in reference_set {
+        reference_set.difference(&current_items).map(|id| id.clone()).collect()
-            if current_items.contains(&original_item) == false {
-                deletions.push(original_item);
-            }
-        }
-        deletions
     }
 }
 
@@ -98,7 +91,7 @@ pub trait CompleteCalendar : PartialCalendar {
     /// Returns the items that have been deleted after `since`
     ///
     /// See also [`PartialCalendar::get_items_deleted_since`]
-    fn get_items_deleted_since(&self, since: DateTime<Utc>) -> Vec<ItemId>;
+    fn get_items_deleted_since(&self, since: DateTime<Utc>) -> HashSet<ItemId>;
 
     /// Returns the list of items that this calendar contains
     fn get_items(&self) -> HashMap<ItemId, &Item>;

src/utils.rs

@@ -1,8 +1,11 @@
 ///! Some utility functions
 
+use std::collections::HashMap;
+
 use minidom::Element;
 
 use crate::traits::CompleteCalendar;
+use crate::calendar::CalendarId;
 
 /// Walks an XML tree and returns every element that has the given name
 pub fn find_elems<S: AsRef<str>>(root: &Element, searched_name: S) -> Vec<&Element> {
@@ -53,13 +56,13 @@ pub fn print_xml(element: &Element) {
 }
 
 /// A debug utility that pretty-prints calendars
-pub fn print_calendar_list<C: CompleteCalendar>(cals: &Vec<C>) {
+pub fn print_calendar_list<C: CompleteCalendar>(cals: &HashMap<CalendarId, C>) {
-    for cal in cals {
+    for (id, cal) in cals {
-        println!("CAL {}", cal.url());
+        println!("CAL {}", id);
         for (_, item) in cal.get_items() {
             let task = item.unwrap_task();
             let completion = if task.completed() {"✓"} else {" "};
-            println!("    {} {}", completion, task.name());
+            println!("    {} {}\t{}", completion, task.name(), task.id());
         }
     }
 }

tests/caldav_client.rs

@@ -42,12 +42,16 @@ async fn test_client() {
     let mut client = Client::new(URL, USERNAME, PASSWORD).unwrap();
     let calendars = client.get_calendars().await.unwrap();
 
+    let mut last_cal = None;
     println!("Calendars:");
     let _ = calendars.iter()
-        .map(|cal| println!("  {}\t{}", cal.name(), cal.url().as_str()))
+        .map(|(id, cal)| {
+            println!("  {}\t{}", cal.name(), id.as_str());
+            last_cal = Some(id);
+        })
         .collect::<()>();
 
-    let _ = client.get_tasks(&calendars[0].url()).await;
+    let _ = client.get_tasks(&last_cal.unwrap()).await;
 }
 
 #[tokio::test]

tests/sync.rs

@@ -46,6 +46,8 @@ async fn populate_test_provider() -> Provider<Cache, CachedCalendar, Cache, Cach
     let mut server = Cache::new(&PathBuf::from(String::from("server.json")));
     let mut local = Cache::new(&PathBuf::from(String::from("local.json")));
 
+    let cal_id = Url::parse("http://todo.list/cal").unwrap();
+
     let task_a = Item::Task(Task::new("task A".into(), Utc.ymd(2000, 1, 1).and_hms(0, 0, 0)));
     let task_b = Item::Task(Task::new("task B".into(), Utc.ymd(2000, 1, 2).and_hms(0, 0, 0)));
     let task_c = Item::Task(Task::new("task C".into(), Utc.ymd(2000, 1, 3).and_hms(0, 0, 0)));
@@ -77,7 +79,7 @@ async fn populate_test_provider() -> Provider<Cache, CachedCalendar, Cache, Cach
 
     // Step 1
     // Build the calendar as it was at the time of the sync
-    let mut calendar = CachedCalendar::new("a list".into(), Url::parse("http://todo.list/cal").unwrap(), my_tasks::calendar::SupportedComponents::TODO);
+    let mut calendar = CachedCalendar::new("a list".into(), cal_id.clone(), my_tasks::calendar::SupportedComponents::TODO);
     calendar.add_item(task_a);
     calendar.add_item(task_b);
     calendar.add_item(task_c);
@@ -97,7 +99,7 @@ async fn populate_test_provider() -> Provider<Cache, CachedCalendar, Cache, Cach
 
     // Step 2
     // Edit the server calendar
-    let cal_server = &mut server.get_calendars_mut().await.unwrap()[0];
+    let cal_server = server.get_calendar_mut(cal_id.clone()).await.unwrap();
 
     cal_server.delete_item(&task_b_id);
 
@@ -124,7 +126,7 @@ async fn populate_test_provider() -> Provider<Cache, CachedCalendar, Cache, Cach
 
     // Step 3
     // Edit the local calendar
-    let cal_local = &mut local.get_calendars_mut().await.unwrap()[0];
+    let cal_local = local.get_calendar_mut(cal_id).await.unwrap();
 
     cal_local.delete_item(&task_c_id);