feat: small refactor of the Downloader

Author: Umatriz

crates/client/src/download.rs

@@ -1,12 +1,12 @@
 use std::{path::PathBuf, sync::Arc};
 
-use anyhow::{anyhow, Context};
+use anyhow::anyhow;
 use egui_task_manager::TaskProgressShared;
 use nomi_core::{
     configs::profile::{Loader, ProfileState},
     downloads::{
         progress::MappedSender,
-        traits::{DownloadResult, Downloader, DownloaderIO, DownloaderIOExt},
+        traits::{DownloadResult, Downloader},
         AssetsDownloader, DownloadQueue,
     },
     game_paths::GamePaths,
@@ -76,10 +76,13 @@ async fn try_download_version(profile: Arc<RwLock<ModdedProfile>>, progress_shar
         let launch_instance = instance.launch_instance(settings, Some(vec!["-Xms2G".to_string(), "-Xmx4G".to_string()]));
 
         let instance = instance.instance();
-        instance.get_io_dyn().io().await?;
+
+        let io = instance.io();
 
         let downloader: Box<dyn Downloader<Data = DownloadResult>> = instance.into_downloader();
 
+        io.await?;
+
         let downloader = DownloadQueue::new().with_downloader_dyn(downloader);
 
         let _ = progress_shared.set_total(downloader.total());
@@ -112,13 +115,15 @@ async fn try_assets(version: String, assets_dir: PathBuf, progress_shared: TaskP
     )
     .await?;
 
-    downloader.get_io().io().await.context("`io` error")?;
+    let io = downloader.io();
 
     let _ = progress_shared.set_total(downloader.total());
 
     let mapped_sender = MappedSender::new_progress_mapper(Box::new(progress_shared.sender()));
 
     Box::new(downloader).download(&mapped_sender).await;
 
+    io.await?;
+
     Ok(())
 }

crates/client/src/states.rs

@@ -2,11 +2,7 @@ use std::{collections::HashSet, path::PathBuf};
 
 use egui_task_manager::{Caller, Task, TaskManager};
 use nomi_core::{
-    downloads::{
-        java::JavaDownloader,
-        progress::MappedSender,
-        traits::{Downloader, DownloaderIO, DownloaderIOExt},
-    },
+    downloads::{java::JavaDownloader, progress::MappedSender, traits::Downloader},
     fs::read_toml_config_sync,
     DOT_NOMI_JAVA_DIR, DOT_NOMI_JAVA_EXECUTABLE, DOT_NOMI_PROFILES_CONFIG, DOT_NOMI_SETTINGS_CONFIG,
 };
@@ -88,14 +84,15 @@ impl JavaState {
 
             let _ = progress.set_total(downloader.total());
 
-            let io = downloader.get_io();
+            let io = downloader.io();
 
             let mapped_sender = MappedSender::new_progress_mapper(Box::new(progress.sender()));
 
             Box::new(downloader).download(&mapped_sender).await;
 
-            io.io().await.report_error();
+            io.await.report_error();
         });
+
         let task = Task::new("Java downloading", caller);
         manager.push_task::<JavaCollection>(task);
     }

crates/nomi-core/src/downloads/downloaders/assets.rs

@@ -11,14 +11,15 @@ use crate::{
         downloaders::file::FileDownloader,
         progress::ProgressSender,
         set::DownloadSet,
-        traits::{DownloadResult, Downloadable, Downloader, DownloaderIO, DownloaderIOExt},
+        traits::{DownloadResult, Downloadable, Downloader},
     },
     fs::write_json_config,
+    PinnedFutureWithBounds,
 };
 
 use super::DownloadQueue;
 
-#[derive(Serialize, Deserialize, Debug)]
+#[derive(Serialize, Deserialize, Debug, Clone)]
 pub struct Assets {
     pub objects: HashMap<String, AssetInformation>,
 }
@@ -113,32 +114,6 @@ impl AssetsDownloader {
     }
 }
 
-impl<'a> DownloaderIOExt<'a> for AssetsDownloader {
-    type IO = AssetsDownloaderIo<'a>;
-
-    fn get_io(&'a self) -> AssetsDownloaderIo<'a> {
-        AssetsDownloaderIo {
-            assets: &self.assets,
-            indexes: self.indexes.clone(),
-            id: self.id.clone(),
-        }
-    }
-}
-
-pub struct AssetsDownloaderIo<'a> {
-    assets: &'a Assets,
-    indexes: PathBuf,
-    id: String,
-}
-
-#[async_trait::async_trait]
-impl DownloaderIO for AssetsDownloaderIo<'_> {
-    async fn io(&self) -> anyhow::Result<()> {
-        let path = self.indexes.join(format!("{}.json", self.id));
-        write_json_config(&self.assets, path).await
-    }
-}
-
 #[async_trait::async_trait]
 impl Downloader for AssetsDownloader {
     type Data = DownloadResult;
@@ -151,4 +126,17 @@ impl Downloader for AssetsDownloader {
     async fn download(self: Box<Self>, sender: &dyn ProgressSender<Self::Data>) {
         Box::new(self.queue).download(sender).await;
     }
+
+    fn io(&self) -> PinnedFutureWithBounds<anyhow::Result<()>> {
+        let id = self.id.clone();
+        let indexes = self.indexes.clone();
+        let assets = self.assets.clone();
+
+        let fut = async move {
+            let path = indexes.join(format!("{id}.json"));
+            write_json_config(&assets, path).await
+        };
+
+        Box::pin(fut)
+    }
 }

crates/nomi-core/src/downloads/downloaders/java.rs

@@ -5,10 +5,10 @@ use std::{
 use thiserror::Error;
 use tracing::error;
 
-use crate::{downloads::progress::ProgressSender, DOT_NOMI_TEMP_DIR};
+use crate::{downloads::progress::ProgressSender, PinnedFutureWithBounds, DOT_NOMI_TEMP_DIR};
 
 use super::{
-    super::traits::{DownloadResult, Downloader, DownloaderIO, DownloaderIOExt},
+    super::traits::{DownloadResult, Downloader},
     FileDownloader,
 };
 
@@ -85,20 +85,27 @@ impl Downloader for JavaDownloader {
 
         Box::new(downloader).download(sender).await;
     }
-}
 
-impl<'a> DownloaderIOExt<'a> for JavaDownloader {
-    type IO = JavaDownloaderIO;
+    fn io(&self) -> PinnedFutureWithBounds<anyhow::Result<()>> {
+        let target_directory = self.target_directory.clone();
 
-    fn get_io(&'a self) -> Self::IO {
-        JavaDownloaderIO {
-            target_directory: self.target_directory.clone(),
-        }
-    }
-}
+        let fut = async move {
+            let path = PathBuf::from(DOT_NOMI_TEMP_DIR).join(consts::ARCHIVE_FILENAME);
+            if !check_hash(path.clone(), consts::SHA256)? {
+                return Err(JavaDownloaderError::HashDoesNotMatch.into());
+            }
 
-pub struct JavaDownloaderIO {
-    target_directory: PathBuf,
+            let file = File::open(&path)?;
+
+            extract(file, &target_directory)?;
+
+            tokio::fs::remove_file(&path).await?;
+
+            Ok(())
+        };
+
+        Box::pin(fut)
+    }
 }
 
 #[cfg(target_os = "windows")]
@@ -117,24 +124,6 @@ fn extract(archive: std::fs::File, target_path: &Path) -> anyhow::Result<()> {
     archive.unpack(target_path).map_err(Into::into)
 }
 
-#[async_trait::async_trait]
-impl DownloaderIO for JavaDownloaderIO {
-    async fn io(&self) -> anyhow::Result<()> {
-        let path = PathBuf::from(DOT_NOMI_TEMP_DIR).join(consts::ARCHIVE_FILENAME);
-        if !check_hash(path.clone(), consts::SHA256)? {
-            return Err(JavaDownloaderError::HashDoesNotMatch.into());
-        }
-
-        let file = File::open(&path)?;
-
-        extract(file, &self.target_directory)?;
-
-        tokio::fs::remove_file(&path).await?;
-
-        Ok(())
-    }
-}
-
 #[derive(Error, Debug)]
 enum JavaDownloaderError {
     #[error("Hash does not match")]

crates/nomi-core/src/downloads/traits.rs

@@ -1,6 +1,8 @@
 use egui_task_manager::Progress;
 
-use super::{downloaders::assets::AssetsDownloaderIo, progress::ProgressSender, DownloadError};
+use crate::PinnedFutureWithBounds;
+
+use super::{progress::ProgressSender, DownloadError};
 
 #[derive(Debug, Clone)]
 pub struct DownloadResult(pub Result<DownloadStatus, DownloadError>);
@@ -41,7 +43,13 @@ pub trait Downloader: Send + Sync {
 
     /// Returns the number of items to download
     fn total(&self) -> u32;
+
     async fn download(self: Box<Self>, sender: &dyn ProgressSender<Self::Data>);
+
+    /// This method must return a pinned future that does not borrow any values from `Self`
+    fn io(&self) -> PinnedFutureWithBounds<anyhow::Result<()>> {
+        Box::pin(async { Ok(()) })
+    }
 }
 
 const _: Option<Box<dyn Downloader<Data = DownloadResult>>> = None;
@@ -62,31 +70,3 @@ where
         sender.update(result).await;
     }
 }
-
-#[async_trait::async_trait]
-pub trait DownloaderIO {
-    async fn io(&self) -> anyhow::Result<()>;
-}
-
-const _: Option<Box<dyn DownloaderIO>> = None;
-
-pub trait DownloaderIOExt<'a> {
-    type IO: DownloaderIO;
-
-    fn get_io(&'a self) -> Self::IO;
-}
-
-const _: Option<Box<dyn DownloaderIOExt<'_, IO = AssetsDownloaderIo<'_>>>> = None;
-
-pub trait ObjectSafeDownloaderIOExt<'a> {
-    fn get_io_dyn(&'a self) -> Box<dyn DownloaderIO + Send + 'a>;
-}
-
-impl<'a, T: DownloaderIOExt<'a>> ObjectSafeDownloaderIOExt<'a> for T
-where
-    T::IO: Send,
-{
-    fn get_io_dyn(&'a self) -> Box<dyn DownloaderIO + Send + 'a> {
-        Box::new(self.get_io())
-    }
-}

crates/nomi-core/src/instance/version_marker.rs

@@ -1,18 +1,16 @@
 use std::fmt::Debug;
 
-use crate::downloads::traits::{DownloadResult, Downloader, ObjectSafeDownloaderIOExt};
+use crate::downloads::traits::{DownloadResult, Downloader};
 
 use super::builder_ext::LaunchInstanceBuilderExt;
 
-pub trait Version:
-    LaunchInstanceBuilderExt + Downloader<Data = DownloadResult> + for<'a> ObjectSafeDownloaderIOExt<'a> + Debug + Send + Sync
-{
+pub trait Version: LaunchInstanceBuilderExt + Downloader<Data = DownloadResult> + Debug + Send + Sync {
     fn into_downloader(self: Box<Self>) -> Box<dyn Downloader<Data = DownloadResult>>;
 }
 
 impl<T> Version for T
 where
-    T: LaunchInstanceBuilderExt + Downloader<Data = DownloadResult> + for<'a> ObjectSafeDownloaderIOExt<'a> + Debug + Send + Sync + 'static,
+    T: LaunchInstanceBuilderExt + Downloader<Data = DownloadResult> + Debug + Send + Sync + 'static,
 {
     fn into_downloader(self: Box<Self>) -> Box<dyn Downloader<Data = DownloadResult>> {
         self

crates/nomi-core/src/lib.rs

@@ -19,12 +19,16 @@ pub mod state;
 
 pub mod consts;
 
+use std::{future::Future, pin::Pin};
+
 pub use consts::*;
 
 pub use regex;
 use sha1::Digest;
 pub use uuid::Uuid;
 
+type PinnedFutureWithBounds<T> = Pin<Box<dyn Future<Output = T> + Send>>;
+
 pub fn calculate_sha1(data: impl AsRef<[u8]>) -> String {
     let value = sha1::Sha1::digest(data);
     base16ct::lower::encode_string(&value)

crates/nomi-core/src/loaders/fabric.rs

@@ -1,4 +1,4 @@
-use std::path::{Path, PathBuf};
+use std::path::PathBuf;
 
 use itertools::Itertools;
 use reqwest::Client;
@@ -11,7 +11,7 @@ use crate::{
             libraries::{LibrariesDownloader, LibrariesMapper},
         },
         progress::ProgressSender,
-        traits::{DownloadResult, Downloader, DownloaderIO, DownloaderIOExt},
+        traits::{DownloadResult, Downloader},
     },
     fs::write_to_file,
     game_paths::GamePaths,
@@ -24,6 +24,7 @@ use crate::{
         simple_lib::SimpleLib,
     },
     state::get_launcher_manifest,
+    PinnedFutureWithBounds,
 };
 
 #[derive(Debug)]
@@ -105,23 +106,6 @@ impl Fabric {
     }
 }
 
-#[allow(clippy::module_name_repetitions)]
-pub struct FabricIO<'a> {
-    profile: &'a FabricProfile,
-    version_path: &'a Path,
-}
-
-#[async_trait::async_trait]
-impl<'a> DownloaderIO for FabricIO<'a> {
-    async fn io(&self) -> anyhow::Result<()> {
-        let path = self.version_path.join(format!("{}.json", self.profile.id));
-
-        let body = serde_json::to_string_pretty(&self.profile)?;
-
-        write_to_file(body.as_bytes(), &path).await
-    }
-}
-
 struct FabricLibrariesMapper {
     libraries: PathBuf,
 }
@@ -146,15 +130,20 @@ impl Downloader for Fabric {
     async fn download(self: Box<Self>, sender: &dyn ProgressSender<Self::Data>) {
         Box::new(self.libraries_downloader).download(sender).await;
     }
-}
 
-impl<'a> DownloaderIOExt<'a> for Fabric {
-    type IO = FabricIO<'a>;
+    fn io(&self) -> PinnedFutureWithBounds<anyhow::Result<()>> {
+        let version_path = self.game_paths.version.clone();
+        let profile = self.profile.clone();
+        let id = self.profile.id.clone();
 
-    fn get_io(&'a self) -> FabricIO<'a> {
-        FabricIO {
-            profile: &self.profile,
-            version_path: &self.game_paths.version,
-        }
+        let fut = async move {
+            let path = version_path.join(format!("{id}.json"));
+
+            let body = serde_json::to_string_pretty(&profile)?;
+
+            write_to_file(body.as_bytes(), &path).await
+        };
+
+        Box::pin(fut)
     }
 }

crates/nomi-core/src/loaders/vanilla.rs

@@ -11,13 +11,14 @@ use crate::{
             libraries::{LibrariesDownloader, LibrariesMapper},
         },
         progress::ProgressSender,
-        traits::{DownloadResult, Downloader, DownloaderIO, DownloaderIOExt},
+        traits::{DownloadResult, Downloader},
         DownloadQueue,
     },
     fs::write_to_file,
     game_paths::GamePaths,
     repository::manifest::{Classifiers, DownloadFile, Library, Manifest},
     state::get_launcher_manifest,
+    PinnedFutureWithBounds,
 };
 
 #[derive(Debug)]
@@ -108,23 +109,6 @@ impl LibrariesMapper<Library> for VanillaNativeLibrariesMapper<'_> {
     }
 }
 
-#[allow(clippy::module_name_repetitions)]
-pub struct VanillaIO<'a> {
-    manifest: &'a Manifest,
-    version_path: &'a Path,
-}
-
-#[async_trait::async_trait]
-impl DownloaderIO for VanillaIO<'_> {
-    async fn io(&self) -> anyhow::Result<()> {
-        let path = self.version_path.join(format!("{}.json", self.manifest.id));
-
-        let body = serde_json::to_string_pretty(&self.manifest)?;
-
-        write_to_file(body.as_bytes(), &path).await
-    }
-}
-
 #[async_trait::async_trait]
 impl Downloader for Vanilla {
     type Data = DownloadResult;
@@ -136,15 +120,20 @@ impl Downloader for Vanilla {
     async fn download(self: Box<Self>, sender: &dyn ProgressSender<Self::Data>) {
         Box::new(self.queue).download(sender).await;
     }
-}
 
-impl<'a> DownloaderIOExt<'a> for Vanilla {
-    type IO = VanillaIO<'a>;
+    fn io(&self) -> PinnedFutureWithBounds<anyhow::Result<()>> {
+        let versions_path = self.game_paths.version.clone();
+        let manifest_id = self.manifest.id.clone();
+        let manifest_res = serde_json::to_string_pretty(&self.manifest);
 
-    fn get_io(&'a self) -> VanillaIO<'a> {
-        VanillaIO {
-            manifest: &self.manifest,
-            version_path: &self.game_paths.version,
-        }
+        let fut = async move {
+            let path = versions_path.join(format!("{manifest_id}.json"));
+
+            let body = manifest_res?;
+
+            write_to_file(body.as_bytes(), &path).await
+        };
+
+        Box::pin(fut)
     }
 }

crates/nomi-core/tests/download_test.rs

@@ -25,10 +25,9 @@ async fn download_test() {
     Box::new(instance.assets().await.unwrap()).download(&tx).await;
 
     let version = instance.instance();
-    {
-        let io = version.get_io_dyn();
-        io.io().await.unwrap();
-    }
 
+    let fut = version.io();
     version.download(&tx).await;
+
+    fut.await.unwrap();
 }

crates/nomi-core/tests/full_fabric_test.rs

@@ -52,10 +52,12 @@ async fn full_fabric_test() {
     Box::new(instance.assets().await.unwrap()).download(&tx).await;
 
     let instance = instance.instance();
-    instance.get_io_dyn().io().await.unwrap();
+    let ui_fut = instance.io();
 
     instance.download(&tx).await;
 
+    ui_fut.await.unwrap();
+
     let profile = VersionProfile::builder()
         .id(1)
         .name("Full-fabric-test".into())