Merge pull request #207 from NobodyXu/refactor

Refactor: Rm global variable `helpers::Client`

Cargo.lock

@@ -147,7 +147,6 @@ dependencies = [
  "log",
  "miette",
  "mimalloc",
- "once_cell",
  "reqwest",
  "scopeguard",
  "semver",

Cargo.toml

@@ -32,7 +32,6 @@ home = "0.5.3"
 log = "0.4.14"
 miette = { version = "5.1.0", features = ["fancy-no-backtrace"] }
 mimalloc = { version = "0.1.29", default-features = false, optional = true }
-once_cell = "1.12.0"
 reqwest = { version = "0.11.11", features = ["rustls-tls", "stream"], default-features = false }
 scopeguard = "1.1.0"
 semver = "1.0.12"

src/drivers/crates_io.rs

@@ -4,6 +4,7 @@ use std::time::Duration;
 use cargo_toml::Manifest;
 use crates_io_api::AsyncClient;
 use log::debug;
+use reqwest::Client;
 use url::Url;
 
 use super::find_version;
@@ -16,6 +17,7 @@ use visitor::ManifestVisitor;
 
 /// Fetch a crate Cargo.toml by name and version from crates.io
 pub async fn fetch_crate_cratesio(
+    client: &Client,
     name: &str,
     version_req: &str,
 ) -> Result<Manifest<Meta>, BinstallError> {
@@ -67,6 +69,7 @@ pub async fn fetch_crate_cratesio(
     let manifest_dir_path: PathBuf = format!("{name}-{version_name}").into();
 
     download_tar_based_and_visit(
+        client,
         Url::parse(&crate_url)?,
         TarBasedFmt::Tgz,
         ManifestVisitor::new(manifest_dir_path),

src/fetchers.rs

@@ -4,6 +4,7 @@ use std::sync::Arc;
 pub use gh_crate_meta::*;
 pub use log::debug;
 pub use quickinstall::*;
+use reqwest::Client;
 
 use crate::{AutoAbortJoinHandle, BinstallError, PkgFmt, PkgMeta};
 
@@ -13,7 +14,7 @@ mod quickinstall;
 #[async_trait::async_trait]
 pub trait Fetcher: Send + Sync {
     /// Create a new fetcher from some data
-    async fn new(data: &Data) -> Arc<Self>
+    async fn new(client: &Client, data: &Data) -> Arc<Self>
     where
         Self: Sized;
 

src/fetchers/gh_crate_meta.rs

@@ -2,6 +2,7 @@ use std::path::Path;
 use std::sync::Arc;
 
 use log::{debug, info, warn};
+use reqwest::Client;
 use reqwest::Method;
 use serde::Serialize;
 use url::Url;
@@ -10,6 +11,7 @@ use super::Data;
 use crate::{download_and_extract, remote_exists, BinstallError, PkgFmt, Template};
 
 pub struct GhCrateMeta {
+    client: Client,
     data: Data,
 }
 
@@ -23,8 +25,11 @@ impl GhCrateMeta {
 
 #[async_trait::async_trait]
 impl super::Fetcher for GhCrateMeta {
-    async fn new(data: &Data) -> Arc<Self> {
+    async fn new(client: &Client, data: &Data) -> Arc<Self> {
-        Arc::new(Self { data: data.clone() })
+        Arc::new(Self {
+            client: client.clone(),
+            data: data.clone(),
+        })
     }
 
     async fn check(&self) -> Result<bool, BinstallError> {
@@ -37,13 +42,13 @@ impl super::Fetcher for GhCrateMeta {
         }
 
         info!("Checking for package at: '{url}'");
-        remote_exists(url, Method::HEAD).await
+        remote_exists(&self.client, url, Method::HEAD).await
     }
 
     async fn fetch_and_extract(&self, dst: &Path) -> Result<(), BinstallError> {
         let url = self.url()?;
         info!("Downloading package from: '{url}'");
-        download_and_extract(url, self.pkg_fmt(), dst).await
+        download_and_extract(&self.client, url, self.pkg_fmt(), dst).await
     }
 
     fn pkg_fmt(&self) -> PkgFmt {

src/fetchers/quickinstall.rs

@@ -2,28 +2,31 @@ use std::path::Path;
 use std::sync::Arc;
 
 use log::{debug, info};
+use reqwest::Client;
 use reqwest::Method;
 use tokio::task::JoinHandle;
 use url::Url;
 
 use super::Data;
-use crate::{download_and_extract, get_reqwest_client, remote_exists, BinstallError, PkgFmt};
+use crate::{download_and_extract, remote_exists, BinstallError, PkgFmt};
 
 const BASE_URL: &str = "https://github.com/alsuren/cargo-quickinstall/releases/download";
 const STATS_URL: &str = "https://warehouse-clerk-tmp.vercel.app/api/crate";
 
 pub struct QuickInstall {
+    client: Client,
     package: String,
     target: String,
 }
 
 #[async_trait::async_trait]
 impl super::Fetcher for QuickInstall {
-    async fn new(data: &Data) -> Arc<Self> {
+    async fn new(client: &Client, data: &Data) -> Arc<Self> {
         let crate_name = &data.name;
         let version = &data.version;
         let target = data.target.clone();
         Arc::new(Self {
+            client: client.clone(),
             package: format!("{crate_name}-{version}-{target}"),
             target,
         })
@@ -33,13 +36,13 @@ impl super::Fetcher for QuickInstall {
         let url = self.package_url();
         self.report();
         info!("Checking for package at: '{url}'");
-        remote_exists(Url::parse(&url)?, Method::HEAD).await
+        remote_exists(&self.client, Url::parse(&url)?, Method::HEAD).await
     }
 
     async fn fetch_and_extract(&self, dst: &Path) -> Result<(), BinstallError> {
         let url = self.package_url();
         info!("Downloading package from: '{url}'");
-        download_and_extract(Url::parse(&url)?, self.pkg_fmt(), dst).await
+        download_and_extract(&self.client, Url::parse(&url)?, self.pkg_fmt(), dst).await
     }
 
     fn pkg_fmt(&self) -> PkgFmt {
@@ -78,6 +81,7 @@ impl QuickInstall {
 
     pub fn report(&self) -> JoinHandle<Result<(), BinstallError>> {
         let stats_url = self.stats_url();
+        let client = self.client.clone();
 
         tokio::spawn(async move {
             if cfg!(debug_assertions) {
@@ -88,7 +92,7 @@ impl QuickInstall {
             let url = Url::parse(&stats_url)?;
             debug!("Sending installation report to quickinstall ({url})");
 
-            get_reqwest_client()
+            client
                 .request(Method::HEAD, url.clone())
                 .send()
                 .await

src/helpers.rs

@@ -7,7 +7,6 @@ use bytes::Bytes;
 use cargo_toml::Manifest;
 use futures_util::stream::Stream;
 use log::debug;
-use once_cell::sync::OnceCell;
 use reqwest::{tls, Client, ClientBuilder, Method, Response};
 use serde::Serialize;
 use tempfile::NamedTempFile;
@@ -50,13 +49,10 @@ pub fn load_manifest_path<P: AsRef<Path>>(
     })
 }
 
-static CLIENT: OnceCell<Client> = OnceCell::new();
+pub fn create_reqwest_client(
-
-/// Should only be called once in main::entry.
-pub fn initialize_reqwest_client(
     secure: bool,
     min_tls: Option<tls::Version>,
-) -> Result<(), BinstallError> {
+) -> Result<Client, BinstallError> {
     const USER_AGENT: &str = concat!(env!("CARGO_PKG_NAME"), "/", env!("CARGO_PKG_VERSION"));
 
     let mut builder = ClientBuilder::new().user_agent(USER_AGENT);
@@ -71,21 +67,15 @@ pub fn initialize_reqwest_client(
         builder = builder.min_tls_version(ver);
     }
 
-    let client = builder.build()?;
+    Ok(builder.build()?)
-
-    CLIENT
-        .set(client)
-        .expect("Reqwest client already initialized");
-
-    Ok(())
 }
 
-pub fn get_reqwest_client() -> &'static Client {
+pub async fn remote_exists(
-    CLIENT.get().expect("Reqwest client is not initialized")
+    client: &Client,
-}
+    url: Url,
-
+    method: Method,
-pub async fn remote_exists(url: Url, method: Method) -> Result<bool, BinstallError> {
+) -> Result<bool, BinstallError> {
-    let req = get_reqwest_client()
+    let req = client
         .request(method.clone(), url.clone())
         .send()
         .await
@@ -94,11 +84,12 @@ pub async fn remote_exists(url: Url, method: Method) -> Result<bool, BinstallErr
 }
 
 async fn create_request(
+    client: &Client,
     url: Url,
 ) -> Result<impl Stream<Item = reqwest::Result<Bytes>>, BinstallError> {
     debug!("Downloading from: '{url}'");
 
-    get_reqwest_client()
+    client
         .get(url.clone())
         .send()
         .await
@@ -113,11 +104,12 @@ async fn create_request(
 
 /// Download a file from the provided URL and extract it to the provided path.
 pub async fn download_and_extract<P: AsRef<Path>>(
+    client: &Client,
     url: Url,
     fmt: PkgFmt,
     path: P,
 ) -> Result<(), BinstallError> {
-    let stream = create_request(url).await?;
+    let stream = create_request(client, url).await?;
 
     let path = path.as_ref();
     debug!("Downloading and extracting to: '{}'", path.display());
@@ -139,11 +131,12 @@ pub async fn download_and_extract<P: AsRef<Path>>(
 ///  * `filter` - If Some, then it will pass the path of the file to it
 ///    and only extract ones which filter returns `true`.
 pub async fn download_tar_based_and_visit<V: TarEntriesVisitor + Debug + Send + 'static>(
+    client: &Client,
     url: Url,
     fmt: TarBasedFmt,
     visitor: V,
 ) -> Result<V::Target, BinstallError> {
-    let stream = create_request(url).await?;
+    let stream = create_request(client, url).await?;
 
     debug!("Downloading and extracting then in-memory processing");
 

src/main.rs

@@ -205,7 +205,7 @@ async fn entry() -> Result<()> {
     };
 
     // Initialize reqwest client
-    initialize_reqwest_client(opts.secure, opts.min_tls_version.map(|v| v.into()))?;
+    let client = create_reqwest_client(opts.secure, opts.min_tls_version.map(|v| v.into()))?;
 
     // Setup logging
     let mut log_config = ConfigBuilder::new();
@@ -244,7 +244,7 @@ async fn entry() -> Result<()> {
     // TODO: support git-based fetches (whole repo name rather than just crate name)
     let manifest = match opts.manifest_path.clone() {
         Some(manifest_path) => load_manifest_path(manifest_path.join("Cargo.toml"))?,
-        None => fetch_crate_cratesio(&opts.name, &opts.version).await?,
+        None => fetch_crate_cratesio(&client, &opts.name, &opts.version).await?,
     };
 
     let package = manifest.package.unwrap();
@@ -298,8 +298,8 @@ async fn entry() -> Result<()> {
             meta: target_meta,
         };
 
-        fetchers.add(GhCrateMeta::new(&fetcher_data).await);
+        fetchers.add(GhCrateMeta::new(&client, &fetcher_data).await);
-        fetchers.add(QuickInstall::new(&fetcher_data).await);
+        fetchers.add(QuickInstall::new(&client, &fetcher_data).await);
     }
 
     match fetchers.first_available().await {