[wip] unit tests for sharry module

- doc + testing for `uri` module

src/appstate.rs

@@ -62,7 +62,7 @@ impl AppState {
         if let Some(upl) = self.inner.peek_uploading() {
             if bar.length().is_none() {
                 bar.set_length(upl.get_size());
-                bar.set_message(upl.get_name().to_owned());
+                bar.set_message(upl.get_name().to_string());
                 bar.enable_steady_tick(Duration::from_millis(100));
             }
 

src/cachefile.rs

@@ -169,23 +169,22 @@ impl CacheFile {
     }
 
     pub fn rewind_chunk(mut self) -> Option<Self> {
-        self.uploading = Some(
-            self.uploading
+        let upl = self
+            .uploading
             .take()
-                .expect("rewind_chunk called while not uploading")
-                .rewind()?,
-        );
+            .expect("rewind_chunk called while not uploading");
 
+        self.uploading = Some(upl.rewind()?);
         Some(self)
     }
 
     pub fn abort_upload(&mut self) {
-        self.files.push_front(
-            self.uploading
+        let upl = self
+            .uploading
             .take()
-                .expect("abort_upload called while not uploading")
-                .abort(),
-        );
+            .expect("abort_upload called while not uploading");
+
+        self.files.push_front(upl.abort());
     }
 
     pub fn share_notify(&self, client: &impl Client) -> crate::Result<()> {

src/cli.rs

@@ -2,11 +2,7 @@ use std::{convert::Infallible, fmt, io, time::Duration};
 
 use base64ct::{Base64UrlUnpadded, Encoding};
 use blake2b_simd::Params as Blake2b;
-use clap::{
-    Parser,
-    builder::{PossibleValuesParser, TypedValueParser},
-    value_parser,
-};
+use clap::{Parser, builder::TypedValueParser, value_parser};
 use log::LevelFilter;
 
 use crate::{
@@ -25,14 +21,6 @@ pub struct Cli {
     )]
     timeout: Duration,
 
-    /// Protocol for Sharry instance
-    #[arg(
-        short, long,
-        default_value = "https", value_name = "VARIANT",
-        value_parser = PossibleValuesParser::new(["http", "https"]),
-    )]
-    protocol: String,
-
     /// Number of times actions are retried
     #[arg(short, long, default_value_t = 5, value_name = "N")]
     retry_limit: u32,
@@ -118,7 +106,7 @@ impl Cli {
 
     #[must_use]
     pub fn get_uri(&self) -> Uri {
-        Uri::new(&self.protocol, &self.url)
+        Uri::from(self.url.clone())
     }
 
     #[must_use]

src/file/uploading.rs

@@ -46,13 +46,11 @@ impl Uploading {
         self.offset
     }
 
-    pub fn rewind(self) -> Option<Self> {
+    pub fn rewind(mut self) -> Option<Self> {
         if let Some(last_offset) = self.last_offset {
-            Some(Self {
-                last_offset: None,
-                offset: last_offset,
-                ..self
-            })
+            self.last_offset = None;
+            self.offset = last_offset;
+            Some(self)
         } else {
             warn!("attempted to rewind twice");
             None

src/lib.rs

@@ -13,3 +13,16 @@ mod sharry;
 pub use appstate::AppState;
 pub use cli::Cli;
 pub use error::{Error, Parameter, Result};
+
+#[cfg(test)]
+#[inline]
+pub fn check_trait<E, A>(expected: &E, actual: &A, tr: &'static str, ty: &'static str)
+where
+    E: std::fmt::Debug + PartialEq<A>,
+    A: std::fmt::Debug,
+{
+    assert_eq!(
+        expected, actual,
+        "`impl {tr} for {ty}` expected: {expected:?}, actual: {actual:?}",
+    );
+}

src/sharry/ids.rs

@@ -30,7 +30,7 @@ impl From<String> for AliasID {
 /// - impl `serde` for cachefile handling
 /// - impl `Display` for formatting compatibility
 #[derive(Debug, Clone, Serialize, Deserialize)]
-pub struct ShareID(String);
+pub struct ShareID(pub(super) String);
 
 impl fmt::Display for ShareID {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -51,7 +51,7 @@ impl From<String> for ShareID {
 /// - impl `Display` for formatting compatibility
 /// - impl `TryFrom<String>` for extracting from matching a "PATCH" uri
 #[derive(Debug, Clone, Serialize, Deserialize)]
-pub struct FileID(String);
+pub struct FileID(pub(super) String);
 
 impl fmt::Display for FileID {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -64,13 +64,13 @@ impl TryFrom<String> for FileID {
 
     fn try_from(value: String) -> crate::Result<Self> {
         /// Pattern breakdown:
-        /// - `^([^:/?#]+)://`        – scheme (anything but `:/?#`) + `"://"`
-        /// - `([^/?#]+)`             – authority/host (anything but `/?#`)
-        /// - `/api/v2/alias/upload/` – literal path segment
-        /// - `([^/]+)`               – capture SID (one or more non-slash chars)
-        /// - `/files/tus/`           – literal path segment
-        /// - `(?P<fid>[^/]+)`        – capture FID (one or more non-slash chars)
-        /// - `$`                     – end of string
+        /// - `^([^:/?#]+)://`          - scheme (anything but `:/?#`) + `"://"`
+        /// - `([^/?#]+)`               - authority/host (anything but `/?#`)
+        /// - `/api/v2/alias/upload/`   - literal path segment
+        /// - `([^/]+)`                 - capture SID (one or more non-slash chars)
+        /// - `/files/tus/`             - literal path segment
+        /// - `(?P<fid>[^/]+)`          - capture FID (one or more non-slash chars)
+        /// - `$`                       - end of string
         static UPLOAD_URL_RE: LazyLock<Regex> = LazyLock::new(|| {
             trace!("compiling UPLOAD_URL_RE");
 
@@ -86,13 +86,13 @@ impl TryFrom<String> for FileID {
             .captures(&value)
             .and_then(|caps| caps.name("fid").map(|m| m.as_str()))
         {
-            let result = Self(fid.to_owned());
+            let result = Self(fid.to_string());
             debug!("{result:?}");
 
             Ok(result)
         } else {
             Err(crate::Error::mismatch(
-                "<proto>://<base>/api/v2/alias/upload/<share>/files/tus/<file>",
+                "<proto>://<host>/api/v2/alias/upload/<share>/files/tus/<file>",
                 value,
             ))
         }
@@ -102,18 +102,7 @@ impl TryFrom<String> for FileID {
 #[cfg(test)]
 mod tests {
     use super::*;
-
-    #[inline]
-    fn check_trait<E, A>(expected: &E, actual: &A, tr: &'static str, ty: &'static str)
-    where
-        E: fmt::Debug + PartialEq<A>,
-        A: fmt::Debug,
-    {
-        assert_eq!(
-            expected, actual,
-            "`impl {tr} for {ty}` expected: {expected:?}, actual: {actual:?}",
-        );
-    }
+    use crate::check_trait;
 
     #[test]
     fn basic_traits_working() {
@@ -128,23 +117,23 @@ mod tests {
         for input in inputs {
             {
                 // check AliasID
-                let _ = AliasID(input.to_owned()); // direct creation
-                let aid = AliasID::from(input.to_owned());
+                let _ = AliasID(input.to_string()); // direct creation
+                let aid = AliasID::from(input.to_string());
                 check_trait(&input, &aid.0, "From<String>", "AliasID");
                 check_trait(&input, &aid.as_ref(), "AsRef<str>", "AliasID");
             }
 
             {
                 // check ShareID
-                let _ = ShareID(input.to_owned()); // direct creation
-                let sid = ShareID::from(input.to_owned());
+                let _ = ShareID(input.to_string()); // direct creation
+                let sid = ShareID::from(input.to_string());
                 check_trait(&input, &sid.0, "From<String>", "ShareID");
                 check_trait(&input, &sid.to_string(), "Display", "ShareID");
             }
 
             {
                 // check FileID
-                let fid = FileID(input.to_owned()); // direct creation
+                let fid = FileID(input.to_string()); // direct creation
                 check_trait(&input, &fid.to_string(), "Display", "FileID");
             }
         }
@@ -169,7 +158,8 @@ mod tests {
         ];
 
         for (good, expected_fid) in cases {
-            let file_id = FileID::try_from(good.to_owned()).expect("URL should parse successfully");
+            let file_id =
+                FileID::try_from(good.to_string()).expect("URL should parse successfully");
             assert_eq!(
                 file_id.0, expected_fid,
                 "Expected `{good}` → FileID({expected_fid}), got {file_id:?}",
@@ -193,12 +183,12 @@ mod tests {
         ];
 
         for bad in bad_inputs {
-            let err = FileID::try_from(bad.to_owned()).expect_err("URL should not parse");
+            let err = FileID::try_from(bad.to_string()).expect_err("URL should not parse");
             // make sure it's the Mismatch variant, and that it contains the original input
             match err {
                 crate::Error::Mismatch { expected, actual } => {
                     assert_eq!(
-                        expected, "<proto>://<base>/api/v2/alias/upload/<share>/files/tus/<file>",
+                        expected, "<proto>://<host>/api/v2/alias/upload/<share>/files/tus/<file>",
                         "Error should output expected format"
                     );
                     assert_eq!(actual, bad, "Error should echo back the input");

src/sharry/uri.rs

@@ -1,6 +1,7 @@
-use std::fmt;
+use std::{fmt, sync::LazyLock};
 
-use log::trace;
+use log::{debug, trace};
+use regex::Regex;
 use serde::{Deserialize, Serialize};
 
 /// ID of a file in a Sharry share
@@ -24,29 +25,70 @@ impl AsRef<[u8]> for Uri {
     }
 }
 
-impl Uri {
-    pub fn new(protocol: impl fmt::Display, base_url: impl fmt::Display) -> Self {
-        Self(format!("{protocol}://{base_url}"))
+impl From<String> for Uri {
+    fn from(value: String) -> Self {
+        fn parse_url(value: &str) -> Option<(String, String)> {
+            /// Pattern breakdown:
+            /// - `^(?P<scheme>[^:/?#]+)://`    - capture scheme (anything but `:/?#`) + `"://"`
+            /// - `(?P<host>[^/?#]+)`           - capture authority/host (anything but `/?#`)
+            /// - `(/.*)?`                      - maybe trailing slash and some path
+            /// - `$`                           - end of string
+            static SHARRY_URI_RE: LazyLock<Regex> = LazyLock::new(|| {
+                trace!("compiling SHARRY_URI_RE");
+
+                Regex::new(r"^(?P<scheme>[^:/?#]+)://(?P<host>[^/?#]+)(/.*)?$")
+                    .expect("Regex compilation failed")
+            });
+
+            SHARRY_URI_RE.captures(value).map(|caps| {
+                let captured = |name| {
+                    caps.name(name)
+                        .expect(&format!("{name} not captured"))
+                        .as_str()
+                        .to_string()
+                };
+
+                (captured("scheme"), captured("host"))
+            })
         }
 
+        trace!("TryFrom {value:?}");
+
+        if let Some((scheme, host)) = parse_url(&value) {
+            let result = Self(format!("{scheme}://{host}"));
+            debug!("{result:?}");
+
+            result
+        } else {
+            Self(value)
+        }
+    }
+}
+
+impl Uri {
+    /// arbitrary endpoint in the Sharry API v2
     fn endpoint(&self, path: fmt::Arguments) -> String {
         let uri = format!("{}/api/v2/{path}", self.0);
         trace!("endpoint: {uri:?}");
         uri
     }
 
+    /// Sharry API endpoint to create a new share
     pub fn share_create(&self) -> String {
         self.endpoint(format_args!("alias/upload/new"))
     }
 
+    /// Sharry API endpoint to ping a share's notification hook
     pub fn share_notify(&self, share_id: &super::ShareID) -> String {
         self.endpoint(format_args!("alias/mail/notify/{share_id}"))
     }
 
+    /// Sharry API endpoint to create a new file inside a share
     pub fn file_create(&self, share_id: &super::ShareID) -> String {
         self.endpoint(format_args!("alias/upload/{share_id}/files/tus"))
     }
 
+    /// Sharry API endpoint to push data into a file inside a share
     pub fn file_patch(&self, share_id: &super::ShareID, file_id: &super::FileID) -> String {
         self.endpoint(format_args!("alias/upload/{share_id}/files/tus/{file_id}"))
     }
@@ -55,31 +97,112 @@ impl Uri {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::{
+        check_trait,
+        sharry::{FileID, ShareID},
+    };
 
     #[test]
-    fn basic_tests() {
+    fn basic_traits_working() {
         let cases = vec![
-            ("http", "example.com", "http://example.com"),
-            ("https", "my-host:8080", "https://my-host:8080"),
-            ("custom+scheme", "host", "custom+scheme://host"),
+            // simple http host
+            "http://example.com",
+            // https host with port
+            "https://my-host:8080",
+            // custom scheme
+            "custom+scheme://host",
         ];
 
-        for (protocol, base_url, display) in cases {
-            let uri = Uri::new(protocol, base_url);
+        for uri_data in cases {
+            let uri = Uri(uri_data.to_string());
+            check_trait(&uri_data, &uri.to_string(), "Display", "Uri");
+            check_trait(&uri_data.as_bytes(), &uri.as_ref(), "AsRef<[u8]>", "Uri");
+        }
+    }
+
+    #[test]
+    fn valid_urls_produce_expected_uri() {
+        let cases = vec![
+            // simple http host
+            ("http://example.com", "http://example.com"),
+            // https host with port
+            ("https://my-host:8080", "https://my-host:8080"),
+            // trailing slash
+            ("scheme://host/", "scheme://host"),
+            // with path
+            ("scheme://host/path/to/whatever", "scheme://host"),
+            // custom scheme
+            ("custom+scheme://host", "custom+scheme://host"),
+        ];
+
+        for (good, expected) in cases {
+            let uri = Uri::from(good.to_string());
+            check_trait(&expected, &uri.0, "From<String>", "Uri");
+        }
+    }
+
+    #[test]
+    fn invalid_urls_passed_through() {
+        let cases = vec![
+            // missing “://”
+            "http:/example.com",
+            // missing scheme
+            "://example.com",
+            // missing host
+            "http://",
+            "ftp://?query",
+            // totally malformed
+            "just-a-string",
+            "",
+            "///",
+        ];
+
+        for bad in cases {
+            let uri = Uri::from(bad.to_string());
+            check_trait(&bad, &uri.0, "From<String>", "Uri");
+        }
+    }
+
+    #[test]
+    fn test_endpoint() {
+        let cases = vec![
+            // simple path
+            ("path/to/something", "/api/v2/path/to/something"),
+            // underscores, hyphens, dots
+            ("bob_smith-son.eve", "/api/v2/bob_smith-son.eve"),
+            // unicode
+            ("漢字ユーザー", "/api/v2/漢字ユーザー"),
+            // empty path
+            ("", "/api/v2/"),
+            // leading/trailing spaces
+            ("  frank  ", "/api/v2/  frank  "),
+            // uppercase
+            ("GUEST", "/api/v2/GUEST"),
+            // numeric
+            ("12345", "/api/v2/12345"),
+        ];
+
+        let uri = Uri("".to_string());
+        for (path, expected) in cases {
+            assert_eq!(&expected, &uri.endpoint(format_args!("{path}")));
+        }
+    }
+
+    #[test]
+    fn test_pub_endpoints() {
+        let uri = Uri("".to_string());
+        let share_id = ShareID("sid".to_string());
+        let file_id = FileID("fid".to_string());
+
+        assert_eq!("/api/v2/alias/upload/new", uri.share_create());
+        assert_eq!("/api/v2/alias/mail/notify/sid", uri.share_notify(&share_id));
         assert_eq!(
-                uri.to_string(),
-                display,
-                "`impl Display for Uri` expected: {:?}, got {:?}",
-                display,
-                uri.to_string(),
+            "/api/v2/alias/upload/sid/files/tus",
+            uri.file_create(&share_id)
         );
         assert_eq!(
-                uri.as_ref(),
-                display.as_bytes(),
-                "`impl AsRef<[u8]> for Uri` expected: {:?}, got {:?}",
-                display.as_bytes(),
-                uri.as_ref(),
+            "/api/v2/alias/upload/sid/files/tus/fid",
+            uri.file_patch(&share_id, &file_id)
         );
     }
 }
-}