From af4805756f8fff7faf000d63478ed6f5b063315c Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Tue, 7 May 2024 13:54:34 +0100 Subject: [PATCH] Changes based on code review --- rust/agama-cli/src/auth.rs | 5 +- rust/agama-cli/src/config.rs | 101 +++++++++++++-------------- rust/agama-lib/src/network/client.rs | 6 +- 3 files changed, 54 insertions(+), 58 deletions(-) diff --git a/rust/agama-cli/src/auth.rs b/rust/agama-cli/src/auth.rs index b0f66fb341..0b669a0856 100644 --- a/rust/agama-cli/src/auth.rs +++ b/rust/agama-cli/src/auth.rs @@ -1,7 +1,6 @@ use clap::{arg, Args, Subcommand}; use reqwest::header::{HeaderMap, HeaderValue, CONTENT_TYPE}; -use rpassword::{prompt_password, read_password}; use std::fs; use std::fs::File; use std::io; @@ -124,7 +123,7 @@ fn jwt_file() -> Option { } /// Path to agama-live token file. fn agama_token_file() -> Option { - Some(home::home_dir()?.join(DEFAULT_AGAMA_TOKEN_FILE)) + home::home_dir().map(|p| p.join(DEFAULT_AGAMA_TOKEN_FILE)) } /// Reads first line from given file @@ -155,7 +154,7 @@ fn read_line_from_file(path: &Path) -> io::Result { /// Asks user to provide a line of input. Displays a prompt. fn read_credential(caption: String) -> io::Result { let caption = format!("{}: ", caption); - let cred = prompt_password(caption.clone()).unwrap(); + let cred = rpassword::prompt_password(caption.clone()).unwrap(); if cred.is_empty() { return Err(io::Error::new( io::ErrorKind::Other, diff --git a/rust/agama-cli/src/config.rs b/rust/agama-cli/src/config.rs index 8b4b10f9a8..627dd82c4d 100644 --- a/rust/agama-cli/src/config.rs +++ b/rust/agama-cli/src/config.rs @@ -1,14 +1,17 @@ -use crate::auth; -use crate::error::CliError; -use crate::printers::{print, Format}; -use agama_lib::connection; -use agama_lib::install_settings::{InstallSettings, Scope}; -use agama_lib::Store as SettingsStore; +use crate::{ + auth, + error::CliError, + printers::{print, Format}, +}; +use agama_lib::{ + connection, + install_settings::{InstallSettings, Scope}, + Store as SettingsStore, +}; use agama_settings::{settings::Settings, SettingObject, SettingValue}; use clap::Subcommand; use convert_case::{Case, Casing}; -use std::str::FromStr; -use std::{collections::HashMap, error::Error, io}; +use std::{collections::HashMap, error::Error, io, str::FromStr}; #[derive(Subcommand, Debug)] pub enum ConfigCommands { @@ -33,56 +36,50 @@ pub enum ConfigAction { } fn token() -> Option { - match auth::jwt() { - Ok(token) => return Some(token), - Err(_) => match auth::agama_token() { - Ok(token) => return Some(token), - Err(_) => return None, - }, - } + auth::jwt().or_else(|_| auth::agama_token()).ok() } pub async fn run(subcommand: ConfigCommands, format: Format) -> anyhow::Result<()> { - if let Some(token) = token() { - let client = agama_lib::http_client(token)?; - let store = SettingsStore::new(connection().await?, client).await?; + let Some(token) = token() else { + println!("You need to login for generating a valid token"); + return Ok(()); + }; - let command = parse_config_command(subcommand)?; - match command { - ConfigAction::Set(changes) => { - let scopes = changes - .keys() - .filter_map(|k| key_to_scope(k).ok()) - .collect(); - let mut model = store.load(Some(scopes)).await?; - for (key, value) in changes { - model.set(&key.to_case(Case::Snake), SettingValue(value))?; - } - Ok(store.store(&model).await?) - } - ConfigAction::Show => { - let model = store.load(None).await?; - print(model, io::stdout(), format)?; - Ok(()) - } - ConfigAction::Add(key, values) => { - let scope = key_to_scope(&key).unwrap(); - let mut model = store.load(Some(vec![scope])).await?; - model.add(&key.to_case(Case::Snake), SettingObject::from(values))?; - Ok(store.store(&model).await?) - } - ConfigAction::Load(path) => { - let contents = std::fs::read_to_string(path)?; - let result: InstallSettings = serde_json::from_str(&contents)?; - let scopes = result.defined_scopes(); - let mut model = store.load(Some(scopes)).await?; - model.merge(&result); - Ok(store.store(&model).await?) + let client = agama_lib::http_client(token)?; + let store = SettingsStore::new(connection().await?, client).await?; + + let command = parse_config_command(subcommand)?; + match command { + ConfigAction::Set(changes) => { + let scopes = changes + .keys() + .filter_map(|k| key_to_scope(k).ok()) + .collect(); + let mut model = store.load(Some(scopes)).await?; + for (key, value) in changes { + model.set(&key.to_case(Case::Snake), SettingValue(value))?; } + Ok(store.store(&model).await?) + } + ConfigAction::Show => { + let model = store.load(None).await?; + print(model, io::stdout(), format)?; + Ok(()) + } + ConfigAction::Add(key, values) => { + let scope = key_to_scope(&key).unwrap(); + let mut model = store.load(Some(vec![scope])).await?; + model.add(&key.to_case(Case::Snake), SettingObject::from(values))?; + Ok(store.store(&model).await?) + } + ConfigAction::Load(path) => { + let contents = std::fs::read_to_string(path)?; + let result: InstallSettings = serde_json::from_str(&contents)?; + let scopes = result.defined_scopes(); + let mut model = store.load(Some(scopes)).await?; + model.merge(&result); + Ok(store.store(&model).await?) } - } else { - println!("You need to login for generating a valid token"); - Ok(()) } } diff --git a/rust/agama-lib/src/network/client.rs b/rust/agama-lib/src/network/client.rs index c1ef278c1e..113ac20110 100644 --- a/rust/agama-lib/src/network/client.rs +++ b/rust/agama-lib/src/network/client.rs @@ -21,7 +21,7 @@ impl NetworkClient { } async fn text_for(&self, response: Response) -> Result { - let status = response.status().clone(); + let status = response.status(); let text = response .text() .await @@ -42,7 +42,7 @@ impl NetworkClient { .await .map_err(|e| ServiceError::NetworkClientError(e.to_string()))?; - Ok(self.text_for(response).await?) + self.text_for(response).await } /// Returns an array of network devices @@ -82,7 +82,7 @@ impl NetworkClient { let id = connection.id.clone(); let response = self.connection(id.as_str()).await; - if let Ok(_) = response { + if response.is_ok() { let path = format!("{API_URL}/connections/{id}"); self.client .put(path)