refactor: use builder pattern (#4220)

src/cli/run.rs

@@ -15,7 +15,7 @@ use crate::config::{Config, SETTINGS};
 use crate::env_diff::EnvMap;
 use crate::errors::Error;
 use crate::file::display_path;
-use crate::task::task_file_providers::TaskFileProviders;
+use crate::task::task_file_providers::TaskFileProvidersBuilder;
 use crate::task::{Deps, GetMatchingExt, Task};
 use crate::toolset::{InstallOptions, ToolsetBuilder};
 use crate::ui::multi_progress_report::MultiProgressReport;
@@ -878,7 +878,9 @@ impl Run {
 
     fn fetch_tasks(&self, tasks: &mut Vec<Task>) -> Result<()> {
         let no_cache = self.no_cache || SETTINGS.task_remote_no_cache.unwrap_or(false);
-        let task_file_providers = TaskFileProviders::new(no_cache);
+        let task_file_providers = TaskFileProvidersBuilder::new()
+            .with_cache(!no_cache)
+            .build();
 
         for t in tasks {
             if let Some(file) = &t.file {

src/task/task_file_providers/mod.rs

@@ -1,36 +1,51 @@
-use std::sync::LazyLock as Lazy;
 use std::{fmt::Debug, path::PathBuf};
 
 mod local_task;
 mod remote_task_http;
 
 pub use local_task::LocalTask;
-pub use remote_task_http::RemoteTaskHttp;
-
-use crate::dirs;
-
-static REMOTE_TASK_CACHE_DIR: Lazy<PathBuf> = Lazy::new(|| dirs::CACHE.join("remote-tasks-cache"));
+use remote_task_http::RemoteTaskHttpBuilder;
 
 pub trait TaskFileProvider: Debug {
     fn is_match(&self, file: &str) -> bool;
     fn get_local_path(&self, file: &str) -> Result<PathBuf, Box<dyn std::error::Error>>;
 }
 
+pub struct TaskFileProvidersBuilder {
+    use_cache: bool,
+}
+
+impl TaskFileProvidersBuilder {
+    pub fn new() -> Self {
+        Self { use_cache: false }
+    }
+
+    pub fn with_cache(mut self, use_cache: bool) -> Self {
+        self.use_cache = use_cache;
+        self
+    }
+
+    pub fn build(self) -> TaskFileProviders {
+        TaskFileProviders::new(self.use_cache)
+    }
+}
+
 pub struct TaskFileProviders {
-    no_cache: bool,
+    use_cache: bool,
 }
 
 impl TaskFileProviders {
-    pub fn new(no_cache: bool) -> Self {
-        Self { no_cache }
+    pub fn new(use_cache: bool) -> Self {
+        Self { use_cache }
     }
 
     fn get_providers(&self) -> Vec<Box<dyn TaskFileProvider>> {
         vec![
-            Box::new(RemoteTaskHttp::new(
-                REMOTE_TASK_CACHE_DIR.clone(),
-                self.no_cache,
-            )),
+            Box::new(
+                RemoteTaskHttpBuilder::new()
+                    .with_cache(self.use_cache)
+                    .build(),
+            ),
             Box::new(LocalTask), // Must be the last provider
         ]
     }
@@ -47,14 +62,14 @@ mod tests {
 
     #[test]
     fn test_get_providers() {
-        let task_file_providers = TaskFileProviders::new(false);
+        let task_file_providers = TaskFileProvidersBuilder::new().build();
         let providers = task_file_providers.get_providers();
         assert_eq!(providers.len(), 2);
     }
 
     #[test]
     fn test_local_file_match_local_provider() {
-        let task_file_providers = TaskFileProviders::new(false);
+        let task_file_providers = TaskFileProvidersBuilder::new().build();
         let cases = vec!["file.txt", "./file.txt", "../file.txt", "/file.txt"];
 
         for file in cases {
@@ -66,7 +81,7 @@ mod tests {
 
     #[test]
     fn test_http_file_match_http_remote_task_provider() {
-        let task_file_providers = TaskFileProviders::new(false);
+        let task_file_providers = TaskFileProvidersBuilder::new().build();
         let cases = vec![
             "http://example.com/file.txt",
             "https://example.com/file.txt",

src/task/task_file_providers/remote_task_http.rs

@@ -1,24 +1,45 @@
 use std::path::PathBuf;
 
-use crate::{env, file, hash, http::HTTP};
+use crate::{dirs, env, file, hash, http::HTTP};
 
 use super::TaskFileProvider;
 
 #[derive(Debug)]
-pub struct RemoteTaskHttp {
-    cache_path: PathBuf,
-    no_cache: bool,
+pub struct RemoteTaskHttpBuilder {
+    store_path: PathBuf,
+    use_cache: bool,
 }
 
-impl RemoteTaskHttp {
-    pub fn new(cache_path: PathBuf, no_cache: bool) -> Self {
+impl RemoteTaskHttpBuilder {
+    pub fn new() -> Self {
         Self {
-            cache_path,
-            no_cache,
+            store_path: env::temp_dir(),
+            use_cache: false,
+        }
+    }
+
+    pub fn with_cache(mut self, use_cache: bool) -> Self {
+        if use_cache {
+            self.store_path = dirs::CACHE.join("remote-http-tasks-cache");
+            self.use_cache = true;
+        }
+        self
+    }
+
+    pub fn build(self) -> RemoteTaskHttp {
+        RemoteTaskHttp {
+            storage_path: self.store_path,
+            is_cached: self.use_cache,
         }
     }
 }
 
+#[derive(Debug)]
+pub struct RemoteTaskHttp {
+    storage_path: PathBuf,
+    is_cached: bool,
+}
+
 impl RemoteTaskHttp {
     fn get_cache_key(&self, file: &str) -> String {
         hash::hash_sha256_to_str(file)
@@ -50,11 +71,11 @@ impl TaskFileProvider for RemoteTaskHttp {
     }
 
     fn get_local_path(&self, file: &str) -> Result<PathBuf, Box<dyn std::error::Error>> {
-        match self.no_cache {
-            false => {
+        match self.is_cached {
+            true => {
                 trace!("Cache mode enabled");
                 let cache_key = self.get_cache_key(file);
-                let destination = self.cache_path.join(&cache_key);
+                let destination = self.storage_path.join(&cache_key);
 
                 if destination.exists() {
                     debug!("Using cached file: {:?}", destination);
@@ -65,7 +86,7 @@ impl TaskFileProvider for RemoteTaskHttp {
                 self.download_file(file, &destination)?;
                 Ok(destination)
             }
-            true => {
+            false => {
                 trace!("Cache mode disabled");
                 let url = url::Url::parse(file)?;
                 let filename = url
@@ -87,13 +108,11 @@ impl TaskFileProvider for RemoteTaskHttp {
 #[cfg(test)]
 mod tests {
 
-    use std::env;
-
     use super::*;
 
     #[test]
     fn test_is_match() {
-        let provider = RemoteTaskHttp::new(env::temp_dir(), true);
+        let provider = RemoteTaskHttpBuilder::new().build();
 
         // Positive cases
         assert!(provider.is_match("http://myhost.com/test.txt"));
@@ -125,7 +144,7 @@ mod tests {
                 .expect(2)
                 .create();
 
-            let provider = RemoteTaskHttp::new(env::temp_dir(), true);
+            let provider = RemoteTaskHttpBuilder::new().build();
             let mock = format!("{}{}", server.url(), request_path);
 
             for _ in 0..2 {
@@ -156,7 +175,7 @@ mod tests {
                 .expect(1)
                 .create();
 
-            let provider = RemoteTaskHttp::new(env::temp_dir(), false);
+            let provider = RemoteTaskHttpBuilder::new().with_cache(true).build();
             let mock = format!("{}{}", server.url(), request_path);
 
             for _ in 0..2 {