Selaa lähdekoodia

Updated anyhow/Result<> for cache.

Steve Thielemann 3 viikkoa sitten
vanhempi
commit
5af94797fc
2 muutettua tiedostoa jossa 130 lisäystä ja 94 poistoa
  1. 128 92
      src/cache.rs
  2. 2 2
      src/main.rs

+ 128 - 92
src/cache.rs

@@ -21,43 +21,6 @@ pub fn relative_to_absolute(base_url: &str, relative_href: &str) -> Result<Strin
     Ok(new_url.to_string())
 }
 
-/*
-Or maybe I should just use the replacements only, so I have some
-idea where the file came from?
-
-It is nice having a clean filename go1.24.1.tar.gz in the cache directory.
- */
-
-/*
-/// Extract filename from the end of a URL.
-///
-/// If this doesn't have a usable path, convert url:
-/// * Remove https:// part
-/// * Replace / with -
-#[must_use]
-pub fn filename_from_url(url: &str) -> Result<String> {
-    let (_, filename) = url.rsplit_once('/').context("Failed to split URL.")?;
-
-    if filename.is_empty() {
-        // Getting the filename part failed.
-        // Like in cases where the url is https://go.dev/dl/
-        // Which becomes go.dev-dl
-        let mut path = url.to_string();
-        path = path.replace("https://", "");
-        path = path.replace("http://", "");
-        path = path.replace("/", "-");
-        if path.ends_with("-") {
-            path.pop();
-        }
-        return Ok(path);
-    }
-    Ok(filename.to_string())
-}
-*/
-
-// Display a number as K, M, G, T, etc.
-// pub fn display_bytes(size: u64) -> String { }
-
 /// Save reqwest::header::HeaderMap to file.
 ///
 /// This also stores the url in the file, so I know what URL was called for
@@ -389,7 +352,7 @@ impl Cache {
     ///
     /// This deletes the .header cache file, which forces a fetch.
     #[allow(dead_code)]
-    pub fn fetch_nocache(&self, url: &str) -> Status {
+    pub fn fetch_nocache(&self, url: &str) -> Result<Status> {
         let mut base = self.filename_for_url(url);
         Self::append_to_filename(&mut base, HEADER_EXT);
         if base.exists() {
@@ -409,7 +372,7 @@ impl Cache {
     ///
     /// This returns Status, which could be Fetched or Cached copy (among other things).
     #[must_use]
-    pub fn fetch(&self, url: &str) -> Status {
+    pub fn fetch(&self, url: &str) -> Result<Status> {
         let base = self.filename_for_url(url);
         /*
         let base = self
@@ -441,17 +404,11 @@ impl Cache {
             }
         };
 
-        let mut result = builder.send().unwrap();
+        let mut result = builder.send()?;
 
         if result.status() == 304 {
             // Cache hit!
-            return Status::Cached(base);
-            /*
-            // println!("Cache hit! 304! {}", url);
-            // base.set_extension("content");
-            let fp = File::open(base.to_str().unwrap()).unwrap();
-            return Status::Cached(fp);
-            */
+            return Ok(Status::Cached(base));
         }
 
         // Ok!  Success!
@@ -474,7 +431,7 @@ impl Cache {
                         // let byte = Byte::from_u64(len);
                         // let adjusted_byte = byte.get_appropriate_unit(UnitType::Binary);
                         // println!("Too Big! {adjusted_byte:.2} {}", url);
-                        return Status::TooBig(len);
+                        return Ok(Status::TooBig(len));
                     }
                 }
             }
@@ -490,33 +447,27 @@ impl Cache {
                     }
                     if !self.accept.contains(&ct.to_string()) {
                         // println!("Unacceptable content-type {} {}", ct, url);
-                        return Status::Unacceptable(ct.to_string());
+                        return Ok(Status::Unacceptable(ct.to_string()));
                     }
                 }
             }
 
-            let r = save_headermap(header_file.to_str().unwrap(), url, result.headers());
-            if r.is_err() {
-                println!("save_headermap: {} {:?}", r.unwrap_err(), header_file);
-            }
+            save_headermap(header_file.to_str().unwrap(), url, result.headers())
+                .context("save_headermap: {header_file}")?;
+
+            let mut fp = File::create(base.to_str().unwrap())?;
+            result.copy_to(&mut fp)?;
 
-            // base.set_extension("content");
-            let mut fp = File::create(base.to_str().unwrap()).unwrap();
-            let _ = result.copy_to(&mut fp);
-            /*
+            /*  // async
             while let Ok(Some(chunk)) = result.chunk().await {
                 let _ = fp.write(&chunk);
             }
             */
-            return Status::Fetched(base);
-            /*
-            let fp = File::open(base.to_str().unwrap()).unwrap();
-            return Status::Fetched(fp);
-            */
+            return Ok(Status::Fetched(base));
         } else {
             // Status error
             // println!("Error {} {}", result.status(), url);
-            return Status::ErrorStatus(u16::from(result.status()));
+            return Ok(Status::ErrorStatus(u16::from(result.status())));
         }
     }
 }
@@ -563,15 +514,18 @@ mod tests {
                 "https://example.com/and/about.html",
             ),
             (
-                ("https://here.com/dir/index.html", "http://there.com/about.html"),
+                (
+                    "https://here.com/dir/index.html",
+                    "http://there.com/about.html",
+                ),
                 "http://there.com/about.html",
             ),
         ]);
         for (base, url) in rel_abs {
             if let Ok(abs) = relative_to_absolute(base.0, base.1) {
-                assert_eq!(abs, url);
+                assert_eq!(abs, url, "Base {}, Rel {} => {}", base.0, base.1, url);
             } else {
-                panic!("Failed {} + {} = {}", base.0, base.1, url);
+                panic!("Failed {} + {} => {}", base.0, base.1, url);
             }
         }
     }
@@ -580,11 +534,52 @@ mod tests {
     fn url_to_filename_test() {
         let mut dir = testdir!();
         dir.push("cache");
+        let temp = dir.clone();
+
         let cache = Cache::new(dir, None).unwrap();
+
         // url_to_basename
-        // filename_for_url
-        // append_to_filename
-        // remove_from_filename
+        let url_base: HashMap<&str, &str> = HashMap::from([
+            ("https://go.dev/dl/go1.23.45.tar.gz", "go1.23.45.tar.gz"),
+            ("https://go.dev/dl", "dl"),
+            ("https://go.dev/dl/", "go-dev-dl"),
+        ]);
+
+        for (url, base) in url_base {
+            // Verify url_to_basename.
+            if let Ok(basename) = Cache::url_to_basename(url) {
+                assert_eq!(base, basename, "{} -> {}", url, base);
+            } else {
+                panic!("filename_for_url({}) failed.", url);
+            }
+            // Verify filename_for_url.
+            let path = cache.filename_for_url(url);
+            let mut newpath = temp.clone();
+            newpath.push(base);
+            assert_eq!(path.as_os_str(), newpath.as_os_str(), "{} -> {}", url, base);
+        }
+
+        for filename in vec!["go1.23.45.tar.gz", "test.html"] {
+            let newname = String::from(filename) + HEADER_EXT;
+            let mut newpath = temp.clone();
+            newpath.set_file_name(filename);
+            Cache::append_to_filename(&mut newpath, HEADER_EXT);
+            assert_eq!(
+                &newpath.file_name().unwrap().to_string_lossy().to_string(),
+                &newname,
+                "{} {}",
+                filename,
+                HEADER_EXT
+            );
+            // Test to make sure this removes HEADER_EXT from the filename.
+            Cache::remove_from_filename(&mut newpath);
+            assert_eq!(
+                &newpath.file_name().unwrap().to_string_lossy().to_string(),
+                filename,
+                "{}",
+                filename
+            )
+        }
     }
 
     #[test]
@@ -601,20 +596,21 @@ mod tests {
 
         t.push("anything");
 
-        if let Status::Fetched(f) = r {
-            assert!(f.exists(), "Cache file exists.");
-            assert_eq!(f, t, "Cache path is what we were expecting.");
-            let mut header_file = t.clone();
-            Cache::append_to_filename(&mut header_file, HEADER_EXT);
-
-            assert!(header_file.exists(), "Cache header file exists.");
-            t.pop();
-            t.push("anything.header");
-            assert_eq!(header_file, t, "Cache header path is what we expected.");
-        } else {
-            panic!("Cache Status is not Status::Fetched, is: {:?}", r);
+        if let Ok(r) = r {
+            if let Status::Fetched(f) = r {
+                assert!(f.exists(), "Cache file exists.");
+                assert_eq!(f, t, "Cache path is what we were expecting.");
+                let mut header_file = t.clone();
+                Cache::append_to_filename(&mut header_file, HEADER_EXT);
+
+                assert!(header_file.exists(), "Cache header file exists.");
+                t.pop();
+                t.push("anything.header");
+                assert_eq!(header_file, t, "Cache header path is what we expected.");
+            } else {
+                panic!("Cache Status is not Status::Fetched, is: {:?}", r);
+            }
         }
-
         // println!("Dir: {:?}, Status: {:?}", t, r); // r has been partially moved.
     }
 
@@ -636,6 +632,8 @@ mod tests {
     #[test]
     #[cfg(feature = "local-httpbin")]
     fn call_local() {
+        use reqwest::Error;
+
         let mut dir = testdir!();
         dir.push("cache");
 
@@ -645,20 +643,25 @@ mod tests {
         let cache = Cache::new(dir, None).unwrap();
         let teapot_url = "http://127.0.0.1/status/418";
 
-        let r = cache.fetch(teapot_url);
-        if let Status::ErrorStatus(code) = r {
-            assert_eq!(code, 418);
+        let r = cache.fetch(&teapot_url);
+        if let Ok(r) = r {
+            if let Status::ErrorStatus(code) = r {
+                assert_eq!(code, 418);
+            } else {
+                panic!("Not an ErrorStatus");
+            }
         } else {
-            panic!("Not an ErrorStatus");
+            panic!("Unexpected error: {r:?}");
         }
-        println!("{:?}", r);
+        // println!("{:?}", r);
+
+        let r = cache.fetch("http://127.0.0.1:1024");
+        assert!(r.is_err(), "Confirm connection error");
 
         /*
         I disabled brotli in the Client builder.
         I get an error below about invalid UTF-8.  The httpbin server isn't smart
         enough to see I don't support it, and sends it anyway.  :(
-
-        Probably because things like brotli (br) are usually only sent over https.
         */
 
         /*
@@ -673,6 +676,18 @@ mod tests {
         */
     }
 
+    /*
+    These tests require a running local httpbin image.
+    ```
+    services:
+      httpbin:
+        image: kennethreitz/httpbin
+        ports:
+          - "80:80"
+    ```
+
+     */
+
     #[test]
     #[cfg(feature = "local-httpbin")]
     fn cache_local() {
@@ -685,8 +700,29 @@ mod tests {
         let cache = Cache::new(dir, None).unwrap();
         let etag_url = "http://127.0.0.1/etag/meow";
 
-        let r = cache.fetch(etag_url);
-        let r2 = cache.fetch(etag_url);
-        println!("{:?}\n{:?}", r, r2);
+        let r = cache.fetch(&etag_url);
+        if let Ok(r) = r {
+            match r {
+                Status::Fetched(_) => {}
+                _ => {
+                    panic!("Expected Status::Fetched on 1st request.");
+                }
+            }
+        } else {
+            panic!("Unexpected error: {r:?}");
+        }
+        // 2nd call, the etag header is set.
+        let r2 = cache.fetch(&etag_url);
+        if let Ok(r2) = r2 {
+            match r2 {
+                Status::Cached(_) => {}
+                _ => {
+                    panic!("Expected Status::Cached on 2nd request.");
+                }
+            }
+        } else {
+            panic!("Unexpected error: {r2:?}");
+        }
+        // println!("{:?}\n{:?}", r, r2);
     }
 }

+ 2 - 2
src/main.rs

@@ -236,7 +236,7 @@ fn main() -> Result<()> {
 
     match &cli.command {
         Some(Commands::Update {}) => {
-            let status = cache.fetch(GO_URL);
+            let status = cache.fetch(GO_URL)?;
 
             // Check to see if file already exists AND check version against go's version.
             // Since the go.dev site doesn't allow caching or knowing when it changes,
@@ -257,7 +257,7 @@ fn main() -> Result<()> {
                             println!("Downloading newer version...");
                             let abs = relative_to_absolute(GO_URL, &relative).unwrap();
 
-                            let latest_status = cache.fetch(&abs);
+                            let latest_status = cache.fetch(&abs)?;
                             if let Some(update) = latest_status.download_path() {
                                 // Ok, we have the update!  Now what?
                                 println!("Ready to install update.");