Merge pull request #1267 from jqnatividad/download_file_refactor

Download file helper refactor
dathere · Aug 31, 2023 · 4a040d8 · 4a040d8
2 parents ee49990 + e7e68a8
commit 4a040d8
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 20 deletions.
diff --git a/src/cmd/geocode.rs b/src/cmd/geocode.rs
@@ -594,24 +594,21 @@ async fn load_engine(geocode_index_file: PathBuf, progressbar: &ProgressBar) ->
 
     if index_file.exists() {
         // load existing local index
-        if !progressbar.is_hidden() {
-            progressbar.println(format!(
-                "Loading existing Geonames index from {}",
-                index_file.display()
-            ));
-        }
+        progressbar.println(format!(
+            "Loading existing Geonames index from {}",
+            index_file.display()
+        ));
     } else {
         // initial load, download index file from qsv releases
-        if !progressbar.is_hidden() {
-            progressbar.println(format!(
-                "Downloading default Geonames index for qsv {QSV_VERSION} release..."
-            ));
-        }
+        progressbar.println(format!(
+            "Downloading default Geonames index for qsv {QSV_VERSION} release..."
+        ));
+
         util::download_file(
             &format!(
                 "https://github.com/jqnatividad/qsv/releases/download/{QSV_VERSION}/qsv-{QSV_VERSION}-geocode-index.bincode"
             ),
-            &geocode_index_file.to_string_lossy(),
+            geocode_index_file.clone(),
             !progressbar.is_hidden(),
             None,
             None,

diff --git a/src/cmd/sample.rs b/src/cmd/sample.rs
@@ -94,17 +94,18 @@ pub fn run(argv: &[&str]) -> CliResult<()> {
         Some(uri) => {
             if Url::parse(&uri).is_ok() && uri.starts_with("http") {
                 // its a remote file, download it first
-                let temp_download_path = temp_download.path().to_str().unwrap().to_string();
-
                 let future = util::download_file(
                     &uri,
-                    &temp_download_path,
+                    temp_download.path().to_path_buf(),
                     false,
                     args.flag_user_agent,
                     args.flag_timeout,
                     None,
                 );
                 tokio::runtime::Runtime::new()?.block_on(future)?;
+                // safety: temp_download is a NamedTempFile, so we know it can be converted to a
+                // string
+                let temp_download_path = temp_download.path().to_str().unwrap().to_string();
                 Some(temp_download_path)
             } else {
                 // its a local file

diff --git a/src/cmd/snappy.rs b/src/cmd/snappy.rs
@@ -106,11 +106,9 @@ pub fn run(argv: &[&str]) -> CliResult<()> {
         Some(uri) => {
             let path = if Url::parse(uri).is_ok() && uri.starts_with("http") {
                 // its a remote file, download it first
-                let temp_download_path = temp_download.path().to_str().unwrap().to_string();
-
                 let future = util::download_file(
                     uri,
-                    &temp_download_path,
+                    temp_download.path().to_path_buf(),
                     args.flag_progressbar && !args.cmd_check && !args.flag_quiet,
                     args.flag_user_agent,
                     Some(args.flag_timeout),
@@ -121,6 +119,8 @@ pub fn run(argv: &[&str]) -> CliResult<()> {
                     },
                 );
                 tokio::runtime::Runtime::new()?.block_on(future)?;
+                // safety: temp_download is a NamedTempFile, so we know that it can be converted
+                let temp_download_path = temp_download.path().to_str().unwrap().to_string();
                 temp_download_path
             } else {
                 // its a local file

diff --git a/src/util.rs b/src/util.rs
@@ -1293,7 +1293,7 @@ pub fn decompress_snappy_file(
 /// if sample_size is Some, it will be used as the number of bytes to download
 pub async fn download_file(
     url: &str,
-    path: &str,
+    path: PathBuf,
     show_progress: bool,
     custom_user_agent: Option<String>,
     download_timeout: Option<u16>,
@@ -1368,7 +1368,7 @@ pub async fn download_file(
     let sample_size = sample_size.unwrap_or(0);
 
     // download chunks
-    let mut file = File::create(path).map_err(|_| format!("Failed to create file '{path}'"))?;
+    let mut file = File::create(path)?;
     let mut downloaded: u64 = 0;
     let mut stream = res.bytes_stream();