Add more unit tests for the config loader

1. Load exact match, supplement "[all]" table
2. Load no match, fall back to "[all]"
3. Load exact match, ignore missing "[all]" table

src/cli.rs

@@ -4,9 +4,9 @@ use clap::{Parser, Subcommand};
 #[command(version, about, long_about = None)]
 pub struct Args {
     #[arg(short = 'u', long = "url", env = "GTTOOL_GITEA_URL")]
-    pub gitea_url: String,
+    pub gitea_url: Option<String>,
     #[arg(short = 'r', long = "repo", env = "GTTOOL_FQRN")]
-    pub repo: String,
+    pub repo: Option<String>,
 
     #[command(subcommand)]
     pub command: Commands,

src/config.rs

@@ -75,7 +75,7 @@ pub fn default_paths() -> impl Iterator<Item = PathBuf> {
 /// TODO: Check for, and warn or error when given a dir.
 pub fn get_config(
     project: &str,
-    search_files: impl Iterator<Item=PathBuf>
+    search_files: impl Iterator<Item = PathBuf>,
 ) -> Result<PartialConfig> {
     /*
     1. Get conf search (from fn input)
@@ -87,7 +87,6 @@ pub fn get_config (
     7. (merge, again) Fold the PartialConfigs into a finished one
     */
 
-
     let file_iter = search_files;
 
     let toml_iter = file_iter
@@ -104,7 +103,8 @@ pub fn get_config (
             let cfg_table = val.as_table().ok_or(Error::BadFormat)?;
 
             // 2. Get table
-            let maybe_proj = get_table(cfg_table, project)
+
+            get_table(cfg_table, project)
                 // 3. convert to PartialConfig
                 .and_then(PartialConfig::try_from)
                 // 4. assemble a 2-tuple of PartialConfigs by...
@@ -116,27 +116,21 @@ pub fn get_config (
                         get_table(cfg_table, "all").and_then(PartialConfig::try_from)?,
                     ))
                 })
-                // 5. Merge the PartialConfigs together, project-specific has higher priority
+                .map(|pair| pair.0.merge(pair.1))
-                .and_then(|pair| {
-                    Ok(pair.0.merge(pair.1))
-                });
-            maybe_proj
         })
         .filter_map(|res| res.ok())
-        .fold(PartialConfig::default(), |acc, inc|{
+        .fold(PartialConfig::default(), |acc, inc| acc.merge(inc));
-            acc.merge(inc)
+    Ok(config_iter)
-        });
-    return Ok(config_iter);
 }
 
 #[derive(Debug, Default)]
 #[cfg_attr(test, derive(PartialEq))]
 pub struct PartialConfig {
     project_path: Option<String>,
-    gitea_url: Option<String>,
+    pub gitea_url: Option<String>,
-    owner: Option<String>,
+    pub owner: Option<String>,
-    repo: Option<String>,
+    pub repo: Option<String>,
-    token: Option<String>,
+    pub token: Option<String>,
 }
 
 impl PartialConfig {
@@ -171,84 +165,34 @@ impl TryFrom<&Table> for PartialConfig {
         Ok(Self {
             // can't get table name because that key is gone by this point.
             project_path: None,
-            gitea_url: get_maybe_property(&value, "gitea_url")?.cloned(),
+            gitea_url: get_maybe_property(value, "gitea_url")?.cloned(),
-            owner: get_maybe_property(&value, "owner")?.cloned(),
+            owner: get_maybe_property(value, "owner")?.cloned(),
-            repo: get_maybe_property(&value, "repo")?.cloned(),
+            repo: get_maybe_property(value, "repo")?.cloned(),
-            token: get_maybe_property(&value, "token")?.cloned(),
+            token: get_maybe_property(value, "token")?.cloned(),
         })
     }
-
-}
-
-#[derive(Debug, Default)]
-#[cfg_attr(test, derive(PartialEq))]
-struct WholeFile {
-    all: PartialConfig,
-    project_overrides: Vec<PartialConfig>,
-}
-
-fn load_from_file(path: &str) -> Result<WholeFile> {
-    let res = std::fs::read_to_string(path);
-    match res {
-        Ok(s) => read_conf_str(s.as_str()),
-        Err(e) => {
-            eprintln!("->> file io err: {:?}", e);
-            Err(Error::CouldntReadFile)
-        }
-    }
-}
-
-fn read_conf_str(text: &str) -> Result<WholeFile> {
-    let mut whole = WholeFile::default();
-
-    let toml_val = text.parse::<Value>()?;
-
-    // The config file is one big table. If the string we decoded is
-    // some other toml::Value variant, it's not correct.
-    // Try getting it as a table, return Err(BadFormat) otherwise.
-    let cfg_table = toml_val.as_table().ok_or(Error::BadFormat)?;
-
-    // Get the global config out of the file
-    let table_all = get_table(cfg_table, "all")?;
-    whole.all = PartialConfig::try_from(table_all)?;
-
-    // Loop over the per-project configs, if any.
-    let per_project_keys = cfg_table
-        .keys()
-        .filter(|s| { // Discard the "[all]" table
-            *s != "all"
-        });
-
-    for path in per_project_keys {
-        let tab = get_table(cfg_table, path)?;
-        let part_cfg = PartialConfig::try_from(tab)?
-            .project_path(path.clone());
-        whole.project_overrides.push(part_cfg);
-    }
-    Ok(whole)
 }
 
 /// The outer value must be a Table so we can get the sub-table from it.
-fn get_table<'outer>(outer: &'outer Table, table_name: impl ToString) -> Result<&'outer Table> {
+fn get_table(outer: &Table, table_name: impl ToString) -> Result<&Table> {
-    Ok(outer
+    outer
         .get(&table_name.to_string())
         .ok_or(Error::NoSuchTable)?
         .as_table()
-        .ok_or(Error::BadFormat)?)
+        .ok_or(Error::BadFormat)
 }
 
-
 /// Similar to `get_property()` but maps the "Error::NoSuchProperty" result to
 /// Option::None. Some properties aren't specified, and that's okay... sometimes.
-fn get_maybe_property<'outer> (outer: &'outer Table, property: impl ToString) -> Result<Option<&'outer String>> {
+fn get_maybe_property(outer: &Table, property: impl ToString) -> Result<Option<&String>> {
     let maybe_prop = get_property(outer, property);
     match maybe_prop {
         Ok(value) => Ok(Some(value)),
         Err(e) => {
             if let Error::NoSuchProperty = e {
-                return Ok(None);
+                Ok(None)
             } else {
-                return Err(e);
+                Err(e)
             }
         }
     }
@@ -256,8 +200,10 @@ fn get_maybe_property<'outer> (outer: &'outer Table, property: impl ToString) ->
 
 /// The config properties are individual strings. This gets the named property,
 /// or an error explaining why it couldn't be fetched.
-fn get_property<'outer>(outer: &'outer Table, property: impl ToString) -> Result<&'outer String> {
+fn get_property(outer: &Table, property: impl ToString) -> Result<&String> {
-    let maybe_prop = outer.get(&property.to_string()).ok_or(Error::NoSuchProperty)?;
+    let maybe_prop = outer
+        .get(&property.to_string())
+        .ok_or(Error::NoSuchProperty)?;
     if let Value::String(text) = maybe_prop {
         Ok(text)
     } else {
@@ -271,43 +217,6 @@ mod tests {
 
     use super::*;
 
-    // Util for generating a reference struct
-    fn gen_expected_struct() -> WholeFile {
-        WholeFile {
-            all: PartialConfig {
-                project_path: None,
-                gitea_url: Some(String::from("http://localhost:3000")),
-                owner: None,
-                repo: None,
-                token: Some(String::from("fake-token"))
-            },
-            project_overrides: vec![
-                PartialConfig {
-                    project_path: Some(String::from("/home/robert/projects/gt-tool")),
-                    gitea_url: None,
-                    owner: Some(String::from("robert")),
-                    repo: Some(String::from("gt-tool")),
-                    token: None,
-                },
-                PartialConfig {
-                    project_path: Some(String::from("/home/robert/projects/rcalc")),
-                    gitea_url: None,
-                    owner: Some(String::from("jamis")),
-                    repo: Some(String::from("rcalc")),
-                    token: None,
-                },
-                PartialConfig {
-                    project_path: Some(String::from("/home/robert/projects/rcalc-builders")),
-                    gitea_url: None,
-                    owner: Some(String::from("jamis")),
-                    repo: Some(String::from("rcalc")),
-                    token: None,
-                },
-            
-            ],
-        }
-    }
-
     #[test]
     fn read_single_prop() -> Result<()> {
         let fx_input_str = "owner = \"dingus\"";
@@ -339,7 +248,10 @@ mod tests {
         let fx_value = fx_input_str.parse::<Value>()?;
         let fx_value = fx_value.as_table().ok_or(Error::BadFormat)?;
         let mut expected: Map<String, Value> = Map::new();
-        expected.insert(String::from("with_a_garbage"), Value::String(String::from("value")));
+        expected.insert(
+            String::from("with_a_garbage"),
+            Value::String(String::from("value")),
+        );
 
         let res = get_table(&fx_value, String::from("tab"))?;
         assert_eq!(res, &expected);
@@ -347,69 +259,11 @@ mod tests {
     }
 
     #[test]
-    fn read_config_string_ok() -> Result<()> {
+    fn test_get_config_with_specific_match() -> Result<()> {
-        let fx_sample_config_string = include_str!("../test_data/sample_config.toml");
-        let fx_expected_struct = gen_expected_struct();
-        let conf = read_conf_str(fx_sample_config_string)?;
-        
-        assert_eq!(conf, fx_expected_struct);
-        Ok(())
-    }
-
-    /* TODO: Improve semantics around reading an empty string
-    An empty config string will result in Error::NoSuchTable when "[all]"
-    is retrieved. But this will *also* happen when other configs are present,
-    but "[all]" isn't. Do I treat these as valid configurations, using some
-    hard-coded default as the fallback? Or do I reject configs that don't have
-    an all-table?
-     */
-    #[test]
-    fn read_config_string_empty() {
-        let fx_sample_cfg = "";
-        let conf = read_conf_str(fx_sample_cfg);
-        assert!(conf.is_err());
-    }
-
-    #[test]
-    // File exists and has valid configuration.
-    fn load_from_file_ok() -> Result<()> {
-        let conf = load_from_file("test_data/sample_config.toml")?;
-        assert_eq!(conf, gen_expected_struct());
-        Ok(())
-    }
-
-    #[test]
-    // File does not exist.
-    fn load_from_file_missing() -> Result<()> {
-        let res = load_from_file("test_data/doesnt_exist.toml");
-        let err = res.unwrap_err();
-        assert_eq!(err, Error::CouldntReadFile);
-        Ok(())
-    }
-
-    #[test]
-    // File exists but has garbage inside.
-    // TODO: This bumps against the same semantic issue as the todo note on
-    // the 'read_config_string_empty' test
-    fn load_from_file_bad() {
-        let res = load_from_file("test_data/missing_all_table.toml");
-        assert!(res.is_err());
-    }
-
-    #[test]
-    // FIXME: Allow user-specified search paths, then update test to use them.
-    // This test can only work if there's a config file the program can find.
-    // Right now, that means a file called "gt-tool.toml" in
-    // 1. `/etc`
-    // 2. anything inside `$XDG_CONFIG_DIRS`
-    fn check_get_config() -> Result<()>{
         let search_paths = ["./test_data/sample_config.toml"]
             .into_iter()
             .map(PathBuf::from);
-        let load_result = get_config(
+        let load_result = get_config("/home/robert/projects/gt-tool", search_paths)?;
-            "/home/robert/projects/gt-tool",
-            search_paths
-        )?;
         let expected = PartialConfig {
             project_path: Some(String::from("/home/robert/projects/gt-tool")),
             owner: Some(String::from("robert")),
@@ -421,4 +275,39 @@ mod tests {
         assert_eq!(load_result, expected);
         Ok(())
     }
+
+    // Ensure the config comes back with something even when there is no
+    // matching project-specific table.
+    #[test]
+    fn test_get_config_no_specific_match() -> Result<()> {
+        let search_paths = ["./test_data/sample_config.toml"]
+            .into_iter()
+            .map(PathBuf::from);
+        let load_result = get_config("/no/such/project", search_paths)?;
+        let expected = PartialConfig {
+            project_path: None,
+            owner: None,
+            repo: None,
+            gitea_url: Some(String::from("http://localhost:3000")),
+            token: Some(String::from("fake-token")),
+        };
+        assert_eq!(load_result, expected);
+        Ok(())
+    }
+
+    // Ensure the config comes back with something even when there is no
+    // "[all]" table
+    #[test]
+    fn test_get_config_without_all() -> Result<()> {
+        let search_paths = ["./test_data/missing_all_table.toml"]
+            .into_iter()
+            .map(PathBuf::from);
+        let load_result = get_config("/some/other/path", search_paths)?;
+        let expected = PartialConfig {
+            gitea_url: Some(String::from("fake-url")),  
+            ..PartialConfig::default() 
+        };
+        assert_eq!(load_result, expected);
+        Ok(())
+    }
 }

src/lib.rs

@@ -21,6 +21,9 @@ pub(crate) async fn decode_client_error(response: reqwest::Response) -> Result<A
 #[derive(Debug)]
 pub enum Error {
     Placeholder, // TODO: Enumerate error modes
+    MissingGiteaUrl, // the gitea URL wasn't specified on the CLI, env, or config file.
+    MissingRepoFRQN, // either the owner, repo, or both weren't specified in the loaded PartialConfig
+    WrappedConfigErr(config::Error),
     WrappedReqwestErr(reqwest::Error),
     MissingAuthToken,
     NoSuchFile, // for release attachment 'file exists' pre-check.
@@ -34,4 +37,10 @@ impl From<reqwest::Error> for crate::Error {
     }
 }
 
+impl From<crate::config::Error> for crate::Error {
+    fn from(value: crate::config::Error) -> Self {
+        Self::WrappedConfigErr(value)
+    }
+}
+
 type Result<T> = core::result::Result<T, Error>;

src/main.rs

@@ -12,6 +12,34 @@ use reqwest::header::ACCEPT;
 async fn main() -> Result<(), gt_tool::Error> {
     let args = Args::parse();
 
+    // TODO: Heuristics to guess project path
+    // See issue #8: https://git.gelvin.dev/robert/gt-tool/issues/8
+    let pwd = std::env::current_dir()
+        .map_err(|_e| gt_tool::Error::WrappedConfigErr(
+            gt_tool::config::Error::CouldntReadFile
+        ))?;
+    let config = gt_tool::config::get_config(
+        pwd.to_str().expect("I assumed the path can be UTF-8, but that didn't work out..."),
+        gt_tool::config::default_paths()
+    )?;
+    println!("->> Loaded Config: {config:?}");
+    // arg parser also checks the environment. Prefer CLI/env, then config file.
+    let gitea_url = args.gitea_url.or(config.gitea_url).ok_or(gt_tool::Error::MissingGiteaUrl)?;
+    // Config files split the repo FQRN into "owner" and "repo" (confusing naming, sorry)
+    // These must be merged back together and passed along.
+    let conf_fqrn = config.owner
+        .ok_or(gt_tool::Error::MissingRepoFRQN)
+        .and_then(| mut own| {
+            let repo = config.repo.ok_or(gt_tool::Error::MissingRepoFRQN)?;
+            own.push_str("/");
+            own.push_str(&repo);
+            Ok(own)
+        });
+    let repo_fqrn  = args.repo
+        .ok_or(gt_tool::Error::MissingRepoFRQN)
+        .or(conf_fqrn)?;
+
+
     let mut headers = reqwest::header::HeaderMap::new();
     headers.append(ACCEPT, header::HeaderValue::from_static("application/json"));
 
@@ -28,7 +56,7 @@ async fn main() -> Result<(), gt_tool::Error> {
     match args.command {
         gt_tool::cli::Commands::ListReleases => {
             let releases =
-                gt_tool::api::release::list_releases(&client, &args.gitea_url, &args.repo).await?;
+                gt_tool::api::release::list_releases(&client, &gitea_url, &repo_fqrn).await?;
             // Print in reverse order so the newest items are closest to the
             // user's command prompt. Otherwise the newest item scrolls off the
             // screen and can't be seen.
@@ -54,7 +82,7 @@ async fn main() -> Result<(), gt_tool::Error> {
                 tag_name,
                 target_commitish,
             };
-            gt_tool::api::release::create_release(&client, &args.gitea_url, &args.repo, submission)
+            gt_tool::api::release::create_release(&client, &gitea_url, &repo_fqrn, submission)
                 .await?;
         }
         gt_tool::cli::Commands::UploadRelease {
@@ -75,7 +103,7 @@ async fn main() -> Result<(), gt_tool::Error> {
             // Grab all, find the one that matches the input tag.
             // Scream if there are multiple matches.
             let release_candidates =
-                gt_tool::api::release::list_releases(&client, &args.gitea_url, &args.repo).await?;
+                gt_tool::api::release::list_releases(&client, &gitea_url, &repo_fqrn).await?;
 
             if let Some(release) = match_release_by_tag(&tag_name, release_candidates) {
                 for file in &files {
@@ -94,8 +122,8 @@ async fn main() -> Result<(), gt_tool::Error> {
                 for file in files {
                     let _attach_desc = gt_tool::api::release_attachment::create_release_attachment(
                         &client,
-                        &args.gitea_url,
+                        &gitea_url,
-                        &args.repo,
+                        &repo_fqrn,
                         release.id,
                         file,
                     )