[optim] Download 30 added items at once in a single HTTP request from the server

Cargo.lock

@@ -113,6 +113,12 @@ dependencies = [
  "serde",
 ]
 
+[[package]]
+name = "either"
+version = "1.6.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e78d4f1cc4ae33bbfc157ed5d5a5ef3bc29227303d595861deb238fcec4e9457"
+
 [[package]]
 name = "encoding_rs"
 version = "0.8.29"
@@ -381,6 +387,15 @@ version = "2.3.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "68f2d64f2edebec4ce84ad108148e67e1064789bee435edc5b60ad398714a3a9"
 
+[[package]]
+name = "itertools"
+version = "0.10.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a9a9d19fa1e79b6215ff29b9d6880b706147f16e9b1dbb1e4e5947b5b02bc5e3"
+dependencies = [
+ "either",
+]
+
 [[package]]
 name = "itoa"
 version = "0.4.8"
@@ -407,6 +422,7 @@ dependencies = [
  "env_logger",
  "ical-daladim",
  "ics",
+ "itertools",
  "log",
  "minidom",
  "once_cell",

Cargo.toml

@@ -35,3 +35,4 @@ ics = "0.5"
 chrono = { version = "0.4", features = ["serde"] }
 csscolorparser = { version = "0.5", features = ["serde"] }
 once_cell = "1.8"
+itertools = "0.10"

src/provider/mod.rs

@@ -8,6 +8,8 @@ use std::marker::PhantomData;
 use std::sync::{Arc, Mutex};
 use url::Url;
 
+use itertools::Itertools;
+
 use crate::traits::{BaseCalendar, CalDavSource, DavCalendar};
 use crate::traits::CompleteCalendar;
 use crate::item::SyncStatus;
@@ -16,6 +18,14 @@ pub mod sync_progress;
 use sync_progress::SyncProgress;
 use sync_progress::{FeedbackSender, SyncEvent};
 
+/// How many items will be batched in a single HTTP request when downloading from the server
+#[cfg(not(test))]
+const DOWNLOAD_BATCH_SIZE: usize = 30;
+/// How many items will be batched in a single HTTP request when downloading from the server
+#[cfg(test)]
+const DOWNLOAD_BATCH_SIZE: usize = 3;
+
+
 /// A data source that combines two `CalDavSource`s, which is able to sync both sources.
 ///
 /// Usually, you will only need to use a provider between a server and a local cache, that is to say a [`CalDavProvider`](crate::CalDavProvider), i.e. a `Provider<Cache, CachedCalendar, Client, RemoteCalendar>`. \
@@ -388,45 +398,58 @@ where
     }
 
     async fn apply_remote_additions(
-        remote_additions: HashSet<Url>,
+        mut remote_additions: HashSet<Url>,
         cal_local: &mut T,
         cal_remote: &mut U,
         progress: &mut SyncProgress,
         cal_name: &str
     ) {
-        //
+        for batch in remote_additions.drain().chunks(DOWNLOAD_BATCH_SIZE).into_iter() {
-        //
+            Self::apply_some_remote_additions(batch, cal_local, cal_remote, progress, cal_name).await;
-        //
+        }
-        //
+    }
-        //
+
-        // TODO: OPTIM: in the server -> local way, download all the content at once
+    async fn apply_some_remote_additions<I: Iterator<Item = Url>>(
-        //
+        remote_additions: I,
-        for url_add in remote_additions {
+        cal_local: &mut T,
-            progress.debug(&format!("> Applying remote addition {} locally", url_add));
+        cal_remote: &mut U,
-            progress.feedback(SyncEvent::InProgress{
+        progress: &mut SyncProgress,
-                calendar: cal_name.to_string(),
+        cal_name: &str
-                details: Self::item_name(&cal_local, &url_add).await,
+    ) {
-            });
+        progress.debug(&format!("> Applying a batch of remote additions locally") /* too bad Chunks does not implement ExactSizeIterator, that could provide useful debug info. See https://github.com/rust-itertools/itertools/issues/171 */);
-            match cal_remote.get_item_by_url(&url_add).await {
+
+        let list_of_additions: Vec<Url> = remote_additions.map(|url| url.clone()).collect();
+        match cal_remote.get_items_by_url(&list_of_additions).await {
             Err(err) => {
-                    progress.warn(&format!("Unable to get remote item {}: {}. Skipping it.", url_add, err));
+                progress.warn(&format!("Unable to get a batch of items {:?}: {}. Skipping them.", list_of_additions, err));
-                    continue;
             },
-                Ok(item) => match item {
+            Ok(items) => {
+                for item in items {
+                    match item {
                         None => {
-                        progress.error(&format!("Inconsistency: new item {} has vanished from the remote end", url_add));
+                            progress.error(&format!("Inconsistency: an item from the batch has vanished from the remote end"));
                             continue;
                         },
                         Some(new_item) => {
                             if let Err(err) = cal_local.add_item(new_item.clone()).await {
-                            progress.error(&format!("Not able to add item {} to local calendar: {}", url_add, err));
+                                progress.error(&format!("Not able to add item {} to local calendar: {}", new_item.url(), err));
                             }
                         },
-                },
-            }
                     }
                 }
 
+                // Notifying every item at the same time would not make sense. Let's notify only one of them
+                let one_item_name = match list_of_additions.get(0) {
+                    Some(url) => Self::item_name(&cal_local, &url).await,
+                    None => String::from("<unable to get the name of the first batched item>"),
+                };
+                progress.feedback(SyncEvent::InProgress{
+                    calendar: cal_name.to_string(),
+                    details: one_item_name,
+                });
+            },
+        }
+    }
 }
 
 

tests/sync.rs

@@ -145,7 +145,7 @@ impl TestFlavour {
             scenarii: scenarii::scenarii_basic(),
             mock_behaviour: Arc::new(Mutex::new(MockBehaviour{
                 add_item_behaviour: (2,3),
-                get_item_by_url_behaviour: (1,4),
+                get_item_by_url_behaviour: (1,12),
                 ..MockBehaviour::default()
             })),
         }