From a02a871aa1866643ac8b5e42eaca56e134e8ca6a Mon Sep 17 00:00:00 2001
From: morrieinmaas <moritz@animo.id>
Date: Thu, 3 Mar 2022 12:14:42 +0100
Subject: [PATCH] chore: code review feedback

Signed-off-by: morrieinmaas <moritz@animo.id>
---
 cli/src/error.rs                 |  2 ++
 cli/src/modules/configuration.rs | 29 ++++++++++++++++-------------
 cli/src/register.rs              | 23 +++++++----------------
 3 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/cli/src/error.rs b/cli/src/error.rs
index 04b816c7..816a3e90 100644
--- a/cli/src/error.rs
+++ b/cli/src/error.rs
@@ -7,6 +7,7 @@ pub enum Error {
     NoEndpointSupplied,
     NoConfigKey,
     UnqualAmountKeyValue,
+    HomeNotFoundError,
     ConfigExists,
     OsUnknown,
     RequiredAttributes,
@@ -26,6 +27,7 @@ impl Display for Error {
             Error::NoEndpointSupplied => write!(f, "No endpoint supplied. Supply an endpoint either via `--endpoint` or via `--config`."),
             Error::NoConfigKey => write!(f, "Required key does not exist in the configuration file."),
             Error::UnqualAmountKeyValue => write!(f, "Supplies keys and values are not equal in size."),
+            Error::HomeNotFoundError => write!(f, "Unable to find home directory."),
             Error::ConfigExists => write!(f, "Configuration file already exists."),
             Error::OsUnknown => write!(f, "Unknown operating system. Failed to detect OS as windows or unix."),
             Error::NoSubcommandSupplied(subcommand) => write!(f, "No subcommand supplied for {}. Check `aries-cli {} --help for the available options.", subcommand, subcommand),
diff --git a/cli/src/modules/configuration.rs b/cli/src/modules/configuration.rs
index d55caa12..bbdecf2c 100644
--- a/cli/src/modules/configuration.rs
+++ b/cli/src/modules/configuration.rs
@@ -1,5 +1,5 @@
-use std::env;
 use std::path::Path;
+use std::path::PathBuf;
 use std::{fmt, fs};
 
 use clap::Args;
@@ -40,23 +40,14 @@ impl fmt::Display for ConfigurationEnvironment {
 
 // TODO: we should implement `from` so we can use todo and have a cleaner api
 pub async fn parse_configuration_args(options: &ConfigurationOptions, logger: Log) -> Result<()> {
-    let default_config_path;
-    if cfg!(windows) {
-        let home = "C:\\Program Files\\Common Files";
-        default_config_path = Path::new(home).join("aries-cli\\config.ini")
-    } else if cfg!(unix) {
-        let home = env!("HOME");
-        default_config_path = Path::new(home).join(".config/aries-cli/config.ini")
-    } else {
-        return Err(error::Error::OsUnknown.into());
-    };
+    let config_path = get_config_path()?;
     if options.initialize {
-        initialise(&default_config_path)?;
+        initialise(&config_path)?;
         logger.log("Initialised the configuration!");
         return Ok(());
     }
     if options.view {
-        return view(&default_config_path, logger);
+        return view(&config_path, logger);
     }
 
     Err(error::Error::NoFlagSupplied("configuration".to_string()).into())
@@ -93,3 +84,15 @@ fn initialise(path: &Path) -> Result<()> {
 
     Ok(())
 }
+
+pub fn get_config_path() -> Result<PathBuf> {
+    if cfg!(windows) {
+        let home = "C:\\Program Files\\Common Files";
+        Ok(Path::new(home).join("aries-cli\\config.ini"))
+    } else if cfg!(unix) {
+        let home = option_env!("HOME").ok_or_else(|| error::Error::HomeNotFoundError);
+        Ok(Path::new(&home?).join(".config/aries-cli/config.ini"))
+    } else {
+        Err(error::Error::OsUnknown.into())
+    }
+}
diff --git a/cli/src/register.rs b/cli/src/register.rs
index 3029f502..50ce0062 100644
--- a/cli/src/register.rs
+++ b/cli/src/register.rs
@@ -1,8 +1,8 @@
-use std::path::{Path, PathBuf};
+use std::path::PathBuf;
 
 use crate::cli::{Cli, Commands};
 use crate::error::{self, Result};
-use crate::modules::configuration::parse_configuration_args;
+use crate::modules::configuration::{get_config_path, parse_configuration_args};
 use crate::modules::credential_definition::parse_credential_definition_args;
 use crate::modules::credentials::parse_credentials_args;
 use crate::modules::message::parse_message_args;
@@ -65,22 +65,13 @@ fn initialise_agent_from_cli(
     endpoint: Option<String>,
     api_key: Option<String>,
 ) -> Result<CloudAgentPython> {
-    let default_config_path;
-    if cfg!(windows) {
-        let home = "C:\\Program Files\\Common Files";
-        default_config_path = Path::new(home).join("aries-cli\\config.ini")
-    } else if cfg!(unix) {
-        let home = env!("HOME");
-        default_config_path = Path::new(home).join(".config/aries-cli/config.ini")
-    } else {
-        return Err(error::Error::OsUnknown.into());
-    };
-    let config_path = config.unwrap_or(default_config_path);
+    let config_path = get_config_path()?;
+    let config_path = config.unwrap_or(config_path);
     let environment = environment;
 
-    // We cannot infer type of error here with `.into()` as we are async
-    let endpoint_from_config = get_value_from_config(&config_path, &environment, "endpoint")
-        .map_err(|_| Box::new(error::Error::NoEndpointSupplied) as Box<dyn std::error::Error>);
+    let endpoint_from_config: Result<String> =
+        get_value_from_config(&config_path, &environment, "endpoint")
+            .map_err(|_| error::Error::NoEndpointSupplied.into());
     let api_key_from_config = get_value_from_config(&config_path, &environment, "api_key");
 
     let endpoint = match endpoint {