From 9c17f0766043d5f25fa35601f2e05b8e1dc84009 Mon Sep 17 00:00:00 2001 From: daladim Date: Fri, 5 Mar 2021 23:32:42 +0100 Subject: [PATCH] The Big Type Overhaul --- src/cache.rs | 73 +++++++++++++++++---------------- src/calendar/cached_calendar.rs | 50 +++++++++++----------- src/calendar/mod.rs | 3 ++ src/client.rs | 14 ++++--- src/provider.rs | 35 ++++++++-------- src/traits.rs | 37 +++++++---------- src/utils.rs | 11 +++-- tests/caldav_client.rs | 8 +++- tests/sync.rs | 8 ++-- 9 files changed, 122 insertions(+), 117 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 8e3c036..40a3d21 100644 --- a/src/cache.rs +++ b/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, + calendars: HashMap, last_sync: Option>, } @@ -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 { - for cal_r in calendars_r { - if cal_l.url() == cal_r.url() { + if keys_are_the_same(&calendars_l, &calendars_r) == false { + return Ok(false); + } + + 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(left: &HashMap, right: &HashMap) -> bool where - T: Hash + Eq, + T: Hash + Eq + Clone, { - left.len() == right.len() - && left.keys().all(|k| right.contains_key(k)) + if left.len() != right.len() { + return false; + } + + let keys_l: HashSet = left.keys().cloned().collect(); + let keys_r: HashSet = right.keys().cloned().collect(); + keys_l == keys_r } #[async_trait] impl CalDavSource for Cache { - async fn get_calendars(&self) -> Result<&Vec, Box> { + async fn get_calendars(&self) -> Result<&HashMap, Box> { Ok(&self.data.calendars) } - async fn get_calendars_mut(&mut self) -> Result, Box> { - Ok( - self.data.calendars.iter_mut() - .collect() - ) + async fn get_calendars_mut(&mut self) -> Result, Box> { + let mut hm = HashMap::new(); + for (id, val) in self.data.calendars.iter_mut() { + hm.insert(id.clone(), val); } + Ok(hm) + } - async fn get_calendar(&self, url: Url) -> Option<&CachedCalendar> { - for cal in &self.data.calendars { - if cal.url() == &url { - return Some(cal); - } + async fn get_calendar(&self, id: CalendarId) -> Option<&CachedCalendar> { + self.data.calendars.get(&id) } - return None; - } - 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; + async fn get_calendar_mut(&mut self, id: CalendarId) -> Option<&mut CachedCalendar> { + self.data.calendars.get_mut(&id) } } diff --git a/src/calendar/cached_calendar.rs b/src/calendar/cached_calendar.rs index 1c107e6..33769b2 100644 --- a/src/calendar/cached_calendar.rs +++ b/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, + items: HashMap, deleted_items: BTreeMap, 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, - items: Vec::new(), + name, id, supported_components, + items: HashMap::new(), deleted_items: BTreeMap::new(), } } @@ -47,8 +47,8 @@ impl PartialCalendar for CachedCalendar { &self.name } - fn url(&self) -> &Url { - &self.url + fn id(&self) -> &CalendarId { + &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) { - self.items.retain(|i| i.id() != item_id); + fn delete_item(&mut self, item_id: &ItemId) -> Result<(), Box> { + 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>, filter: Option) -> HashMap { @@ -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 { - self.items.iter() - .map(|item| item.id().clone()) - .collect() + fn get_item_ids(&mut self) -> HashSet { + self.items.keys().cloned().collect() } fn get_item_by_id_mut(&mut self, id: &ItemId) -> Option<&mut Item> { - for item in &mut self.items { - if item.id() == id { - return Some(item); - } - } - return None; + self.items.get_mut(id) } } impl CompleteCalendar for CachedCalendar { /// Returns the items that have been deleted after `since` - fn get_items_deleted_since(&self, since: DateTime) -> Vec { + fn get_items_deleted_since(&self, since: DateTime) -> HashSet { self.deleted_items.range(since..) - .map(|(_key, value)| value.clone()) - .collect() + .map(|(_key, id)| id.clone()) + .collect() } /// Returns the list of items that this calendar contains diff --git a/src/calendar/mod.rs b/src/calendar/mod.rs index 2193a0c..68fa9d0 100644 --- a/src/calendar/mod.rs +++ b/src/calendar/mod.rs @@ -61,3 +61,6 @@ impl Default for SearchFilter { SearchFilter::All } } + + +pub type CalendarId = url::Url; diff --git a/src/client.rs b/src/client.rs index 2dd0570..46a4546 100644 --- a/src/client.rs +++ b/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, calendar_home_set: Option, - calendars: Option>, + calendars: Option>, } 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, Box> { + pub async fn get_calendars(&mut self) -> Result, Box> { 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("".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> { + pub async fn get_tasks(&mut self, calendar: &CalendarId) -> Result<(), Box> { let method = Method::from_bytes(b"REPORT") .expect("cannot create REPORT method."); diff --git a/src/provider.rs b/src/provider.rs index c09e4c2..de804fa 100644 --- a/src/provider.rs +++ b/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 { - let cal_local = match self.local.get_calendar_mut(cal_server.url().clone()).await { + for (id, cal_server) in cals_server { + 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 { - Some(_date) => cal_server.find_deletions(cal_local.get_item_ids()), - None => Vec::new(), - }; - let local_del = match last_sync { - Some(date) => cal_local.get_items_deleted_since(date), + // Pull remote changes from the server + let mut tasks_id_to_remove_from_local = match last_sync { 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 mut tasks_to_add_to_server = Vec::new(); + let local_del = match last_sync { + Some(date) => cal_local.get_items_deleted_since(date), + 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); diff --git a/src/traits.rs b/src/traits.rs index 28a48bb..0f994e5 100644 --- a/src/traits.rs +++ b/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 { /// 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, Box>; + async fn get_calendars(&self) -> Result<&HashMap, Box>; /// 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, Box>; + async fn get_calendars_mut(&mut self) -> Result, Box>; // // // 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 - async fn get_calendar(&self, url: Url) -> Option<&T>; - /// Returns the calendar matching the URL - async fn get_calendar_mut(&mut self, url: Url) -> Option<&mut T>; + /// Returns the calendar matching the ID + async fn get_calendar(&self, id: CalendarId) -> Option<&T>; + /// Returns the calendar matching the ID + 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 - fn url(&self) -> &Url; + /// Returns the calendar unique ID + 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; /// Get the IDs of all current items in this calendar - fn get_item_ids(&mut self) -> Vec; + fn get_item_ids(&mut self) -> HashSet; /// 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>; /// 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) -> Vec { - let mut deletions = Vec::new(); - + fn find_deletions_from(&mut self, reference_set: HashSet) -> HashSet { let current_items = self.get_item_ids(); - for original_item in reference_set { - if current_items.contains(&original_item) == false { - deletions.push(original_item); - } - } - deletions + reference_set.difference(¤t_items).map(|id| id.clone()).collect() } } @@ -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) -> Vec; + fn get_items_deleted_since(&self, since: DateTime) -> HashSet; /// Returns the list of items that this calendar contains fn get_items(&self) -> HashMap; diff --git a/src/utils.rs b/src/utils.rs index badc5e1..bb12600 100644 --- a/src/utils.rs +++ b/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>(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(cals: &Vec) { - for cal in cals { - println!("CAL {}", cal.url()); +pub fn print_calendar_list(cals: &HashMap) { + for (id, cal) in cals { + 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()); } } } diff --git a/tests/caldav_client.rs b/tests/caldav_client.rs index defd6fb..afa7d63 100644 --- a/tests/caldav_client.rs +++ b/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] diff --git a/tests/sync.rs b/tests/sync.rs index 4661850..35a02fd 100644 --- a/tests/sync.rs +++ b/tests/sync.rs @@ -46,6 +46,8 @@ async fn populate_test_provider() -> Provider Provider Provider Provider