From 3d9adcec0887743eb0c0f80c2d176fc45f703881 Mon Sep 17 00:00:00 2001 From: daladim Date: Sat, 3 Apr 2021 18:32:59 +0200 Subject: [PATCH] Fixed a SyncStatus inconsistency --- src/calendar/cached_calendar.rs | 50 ++++++++++++++++++++++++++++++--- src/calendar/remote_calendar.rs | 4 +-- src/provider.rs | 20 +++++++++---- src/traits.rs | 7 +++-- src/utils.rs | 3 +- tests/scenarii.rs | 35 ++++++++++++++--------- 6 files changed, 90 insertions(+), 29 deletions(-) diff --git a/src/calendar/cached_calendar.rs b/src/calendar/cached_calendar.rs index 8eb6af9..583a884 100644 --- a/src/calendar/cached_calendar.rs +++ b/src/calendar/cached_calendar.rs @@ -17,10 +17,43 @@ pub struct CachedCalendar { name: String, id: CalendarId, supported_components: SupportedComponents, + #[cfg(feature = "local_calendar_mocks_remote_calendars")] + is_mocking_remote_calendar: bool, items: HashMap, } +impl CachedCalendar { + /// Activate the "mocking remote calendar" feature (i.e. ignore sync statuses, since this is what an actual CalDAV sever would do) + #[cfg(feature = "local_calendar_mocks_remote_calendars")] + pub fn set_is_mocking_remote_calendar(&mut self) { + self.is_mocking_remote_calendar = true; + } + + /// Add an item + async fn regular_add_item(&mut self, item: Item) -> Result> { + // TODO: here (and in the remote version, display an errror in case we overwrite something?) + let ss_clone = item.sync_status().clone(); + log::debug!("Adding an item with {:?}", ss_clone); + self.items.insert(item.id().clone(), item); + Ok(ss_clone) + } + + /// Add an item, but force a "synced" SyncStatus. This is the typical behaviour on a remote calendar + #[cfg(feature = "local_calendar_mocks_remote_calendars")] + async fn add_item_force_synced(&mut self, mut item: Item) -> Result> { + log::debug!("Adding an item, but forces a synced SyncStatus"); + match item.sync_status() { + SyncStatus::Synced(_) => (), + _ => item.set_sync_status(SyncStatus::random_synced()), + }; + let ss_clone = item.sync_status().clone(); + self.items.insert(item.id().clone(), item); + Ok(ss_clone) + } +} + + #[async_trait] impl BaseCalendar for CachedCalendar { fn name(&self) -> &str { @@ -35,10 +68,17 @@ impl BaseCalendar for CachedCalendar { self.supported_components } - async fn add_item(&mut self, item: Item) -> Result<(), Box> { - // TODO: here (and in the remote version, display an errror in case we overwrite something?) - self.items.insert(item.id().clone(), item); - Ok(()) + #[cfg(not(feature = "local_calendar_mocks_remote_calendars"))] + async fn add_item(&mut self, item: Item) -> Result> { + self.regular_add_item(item).await + } + #[cfg(feature = "local_calendar_mocks_remote_calendars")] + async fn add_item(&mut self, item: Item) -> Result> { + if self.is_mocking_remote_calendar { + self.add_item_force_synced(item).await + } else { + self.regular_add_item(item).await + } } } @@ -47,6 +87,8 @@ impl CompleteCalendar for CachedCalendar { fn new(name: String, id: CalendarId, supported_components: SupportedComponents) -> Self { Self { name, id, supported_components, + #[cfg(feature = "local_calendar_mocks_remote_calendars")] + is_mocking_remote_calendar: false, items: HashMap::new(), } } diff --git a/src/calendar/remote_calendar.rs b/src/calendar/remote_calendar.rs index fbaa860..720a4a2 100644 --- a/src/calendar/remote_calendar.rs +++ b/src/calendar/remote_calendar.rs @@ -58,8 +58,8 @@ impl BaseCalendar for RemoteCalendar { } /// Add an item into this calendar - async fn add_item(&mut self, _item: Item) -> Result<(), Box> { - Err("Not implemented".into()) + async fn add_item(&mut self, _item: Item) -> Result> { + todo!() } } diff --git a/src/provider.rs b/src/provider.rs index e453b88..aa077f8 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -247,14 +247,18 @@ where for id_add in local_additions { log::debug!("> Pushing local addition {} to the server", id_add); - match cal_local.get_item_by_id_ref(&id_add).await { + match cal_local.get_item_by_id_mut(&id_add).await { None => { log::error!("Inconsistency: created item {} has been marked for upload but is locally missing", id_add); continue; }, Some(item) => { - if let Err(err) = cal_remote.add_item(item.clone()).await { - log::error!("Unable to add item {} to remote calendar: {}", id_add, err); + match cal_remote.add_item(item.clone()).await { + Err(err) => log::error!("Unable to add item {} to remote calendar: {}", id_add, err), + Ok(new_ss) => { + // Update local sync status + item.set_sync_status(new_ss); + }, } }, }; @@ -262,7 +266,7 @@ where for id_change in local_changes { log::debug!("> Pushing local change {} to the server", id_change); - match cal_local.get_item_by_id_ref(&id_change).await { + match cal_local.get_item_by_id_mut(&id_change).await { None => { log::error!("Inconsistency: modified item {} has been marked for upload but is locally missing", id_change); continue; @@ -271,8 +275,12 @@ where if let Err(err) = cal_remote.delete_item(&id_change).await { log::error!("Unable to delete item {} from remote calendar: {}", id_change, err); } - if let Err(err) = cal_remote.add_item(item.clone()).await { - log::error!("Unable to add item {} to remote calendar: {}", id_change, err); + match cal_remote.add_item(item.clone()).await { + Err(err) => log::error!("Unable to add item {} to remote calendar: {}", id_change, err), + Ok(new_ss) => { + // Update local sync status + item.set_sync_status(new_ss); + }, } } }; diff --git a/src/traits.rs b/src/traits.rs index 89df9e5..503f819 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -4,6 +4,7 @@ use std::sync::{Arc, Mutex}; use async_trait::async_trait; +use crate::SyncStatus; use crate::item::Item; use crate::item::ItemId; use crate::item::VersionTag; @@ -34,8 +35,10 @@ pub trait BaseCalendar { /// Returns the supported kinds of components for this calendar fn supported_components(&self) -> crate::calendar::SupportedComponents; - /// Add an item into this calendar - async fn add_item(&mut self, item: Item) -> Result<(), Box>; + /// Add an item into this calendar, and return its new sync status. + /// For local calendars, the sync status is not modified. + /// For remote calendars, the sync status is updated by the server + async fn add_item(&mut self, item: Item) -> Result>; /// Returns whether this calDAV calendar supports to-do items fn supports_todo(&self) -> bool { diff --git a/src/utils.rs b/src/utils.rs index 43d27f9..d35cc33 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -106,9 +106,10 @@ pub fn print_task(item: &Item) { status += " "; } match task.sync_status() { + SyncStatus::NotSynced => { status += " "; }, + SyncStatus::Synced(_) => { status += "="; }, SyncStatus::LocallyModified(_) => { status += "~"; }, SyncStatus::LocallyDeleted(_) => { status += "x"; }, - _ => (), } println!(" {} {}\t{}", status, task.name(), task.id()); }, diff --git a/tests/scenarii.rs b/tests/scenarii.rs index 7aab1f1..38d1e40 100644 --- a/tests/scenarii.rs +++ b/tests/scenarii.rs @@ -6,7 +6,7 @@ //! This module builds actual CalDAV sources (actually [`crate::cache::Cache`]s, that can also mock what would be [`crate::client::Client`]s in a real program) and [`crate::provider::Provider]`s that contain this data //! //! This module can also check the sources after a sync contain the actual data we expect -#![cfg(feature = "integration_tests")] +#![cfg(feature = "local_calendar_mocks_remote_calendars")] use std::path::PathBuf; use std::sync::{Arc, Mutex}; @@ -388,17 +388,23 @@ pub async fn populate_test_provider_after_sync(scenarii: &[ItemScenario]) -> Pro populate_test_provider(scenarii, true).await } -async fn populate_test_provider(scenarii: &[ItemScenario], final_state: bool) -> Provider { +async fn populate_test_provider(scenarii: &[ItemScenario], populate_for_final_state: bool) -> Provider { let mut remote = Cache::new(&PathBuf::from(String::from("test_cache_remote/"))); let mut local = Cache::new(&PathBuf::from(String::from("test_cache_local/"))); // Create the initial state, as if we synced both sources in a given state for item in scenarii { - let required_state = if final_state { &item.after_sync } else { &item.initial_state }; + let required_state = if populate_for_final_state { &item.after_sync } else { &item.initial_state }; let (state, sync_status) = match required_state { LocatedState::None => continue, - LocatedState::Local(s) => (s, SyncStatus::NotSynced), - LocatedState::Remote(s) => (s, SyncStatus::random_synced()), + LocatedState::Local(s) => { + assert!(populate_for_final_state == false, "You are not supposed to expect an item in this state after sync"); + (s, SyncStatus::NotSynced) + }, + LocatedState::Remote(s) => { + assert!(populate_for_final_state == false, "You are not supposed to expect an item in this state after sync"); + (s, SyncStatus::random_synced()) + } LocatedState::BothSynced(s) => (s, SyncStatus::random_synced()), }; @@ -413,14 +419,14 @@ async fn populate_test_provider(scenarii: &[ItemScenario], final_state: bool) -> match required_state { LocatedState::None => panic!("Should not happen, we've continued already"), LocatedState::Local(s) => { - get_or_insert_calendar(&mut local, &s.calendar).await.unwrap().lock().unwrap().add_item(new_item).await.unwrap(); + get_or_insert_calendar(&mut local, &s.calendar, false).await.unwrap().lock().unwrap().add_item(new_item).await.unwrap(); }, LocatedState::Remote(s) => { - get_or_insert_calendar(&mut remote, &s.calendar).await.unwrap().lock().unwrap().add_item(new_item).await.unwrap(); + get_or_insert_calendar(&mut remote, &s.calendar, true).await.unwrap().lock().unwrap().add_item(new_item).await.unwrap(); }, LocatedState::BothSynced(s) => { - get_or_insert_calendar(&mut local, &s.calendar).await.unwrap().lock().unwrap().add_item(new_item.clone()).await.unwrap(); - get_or_insert_calendar(&mut remote, &s.calendar).await.unwrap().lock().unwrap().add_item(new_item).await.unwrap(); + get_or_insert_calendar(&mut local, &s.calendar, false).await.unwrap().lock().unwrap().add_item(new_item.clone()).await.unwrap(); + get_or_insert_calendar(&mut remote, &s.calendar, true).await.unwrap().lock().unwrap().add_item(new_item).await.unwrap(); }, } } @@ -448,17 +454,18 @@ async fn apply_changes_on_provider(provider: &mut Provider(source: &mut S, id: &CalendarId) -> Result>, Box> -where - S: CalDavSource, - C: CompleteCalendar + DavCalendar, // in this test, we're using a calendar that mocks both kinds +async fn get_or_insert_calendar(source: &mut Cache, id: &CalendarId, should_mock_remote_calendar: bool) + -> Result>, Box> { match source.get_calendar(id).await { Some(cal) => Ok(cal), None => { let new_name = format!("Calendar for ID {}", id); let supported_components = SupportedComponents::TODO; - let cal = C::new(new_name.to_string(), id.clone(), supported_components); + let mut cal = CachedCalendar::new(new_name.to_string(), id.clone(), supported_components); + if should_mock_remote_calendar { + cal.set_is_mocking_remote_calendar(); + } source.insert_calendar(cal).await } }