From 8d55913c724b409961d86154eaa23e6f867c96d5 Mon Sep 17 00:00:00 2001 From: Adrian Benavides Date: Fri, 10 Nov 2023 10:01:29 +0100 Subject: [PATCH] refactor(rust): tuify `relay delete` and `relay show` commands Co-authored-by: PanGan21 Co-authored-by: axd99 --- .../src/nodes/service/background_node.rs | 5 - .../rust/ockam/ockam_command/src/node/show.rs | 4 +- .../ockam/ockam_command/src/relay/delete.rs | 168 +++++++++++----- .../rust/ockam/ockam_command/src/relay/mod.rs | 1 + .../ockam/ockam_command/src/relay/show.rs | 183 ++++++++++++++---- .../ockam/ockam_command/src/relay/util.rs | 10 + .../ockam_command/src/tcp/inlet/delete.rs | 4 +- .../ockam/ockam_command/tests/bats/relay.bats | 28 ++- 8 files changed, 303 insertions(+), 100 deletions(-) create mode 100644 implementations/rust/ockam/ockam_command/src/relay/util.rs diff --git a/implementations/rust/ockam/ockam_api/src/nodes/service/background_node.rs b/implementations/rust/ockam/ockam_api/src/nodes/service/background_node.rs index 8529ab74593..0cdcaacccb8 100644 --- a/implementations/rust/ockam/ockam_api/src/nodes/service/background_node.rs +++ b/implementations/rust/ockam/ockam_api/src/nodes/service/background_node.rs @@ -183,9 +183,4 @@ impl BackgroundNode { let route = self.create_route().await?; Ok(Client::new(&route, timeout)) } - - /// Get the node name - pub fn name(&self) -> &str { - &self.node_name - } } diff --git a/implementations/rust/ockam/ockam_command/src/node/show.rs b/implementations/rust/ockam/ockam_command/src/node/show.rs index b8c350a121f..26ed0a0fd65 100644 --- a/implementations/rust/ockam/ockam_command/src/node/show.rs +++ b/implementations/rust/ockam/ockam_command/src/node/show.rs @@ -103,8 +103,8 @@ impl ShowCommandTui for ShowTui { Ok(()) } - async fn show_multiple(&self, selected_items_names: Vec) -> miette::Result<()> { - let nodes = list::get_nodes_info(&self.ctx, &self.opts, selected_items_names).await?; + async fn show_multiple(&self, items_names: Vec) -> miette::Result<()> { + let nodes = list::get_nodes_info(&self.ctx, &self.opts, items_names).await?; list::print_nodes_info(&self.opts, nodes)?; Ok(()) } diff --git a/implementations/rust/ockam/ockam_command/src/relay/delete.rs b/implementations/rust/ockam/ockam_command/src/relay/delete.rs index b1ac92bcae5..74766d8f1c8 100644 --- a/implementations/rust/ockam/ockam_command/src/relay/delete.rs +++ b/implementations/rust/ockam/ockam_command/src/relay/delete.rs @@ -1,6 +1,7 @@ use clap::Args; use colorful::Colorful; -use miette::{miette, IntoDiagnostic}; +use console::Term; +use miette::miette; use ockam::Context; use ockam_api::nodes::models::relay::RelayInfo; @@ -8,28 +9,27 @@ use ockam_api::nodes::BackgroundNode; use ockam_core::api::Request; use crate::node::get_node_name; +use crate::relay::util::relay_name_parser; +use crate::terminal::tui::DeleteCommandTui; use crate::util::{node_rpc, parse_node_name}; -use crate::{docs, fmt_ok, CommandGlobalOpts}; +use crate::{docs, fmt_ok, fmt_warn, CommandGlobalOpts, Terminal, TerminalStream}; const AFTER_LONG_HELP: &str = include_str!("./static/delete/after_long_help.txt"); /// Delete a Relay #[derive(Clone, Debug, Args)] -#[command( -arg_required_else_help = false, -after_long_help = docs::after_help(AFTER_LONG_HELP) -)] +#[command(after_long_help = docs::after_help(AFTER_LONG_HELP))] pub struct DeleteCommand { - /// Name assigned to Relay that will be deleted - #[arg(display_order = 900, required = true)] - relay_name: String, + /// Name assigned to the Relay, prefixed with 'forward_to_'. Example: 'forward_to_myrelay' + #[arg(value_parser = relay_name_parser)] + relay_name: Option, /// Node on which to delete the Relay. If not provided, the default node will be used #[arg(global = true, long, value_name = "NODE")] pub at: Option, /// Confirm the deletion without prompting - #[arg(display_order = 901, long, short)] + #[arg(long, short)] yes: bool, } @@ -43,43 +43,121 @@ pub async fn run_impl( ctx: Context, (opts, cmd): (CommandGlobalOpts, DeleteCommand), ) -> miette::Result<()> { - let relay_name = cmd.relay_name.clone(); - let at = get_node_name(&opts.state, &cmd.at); - let node_name = parse_node_name(&at)?; - let node = BackgroundNode::create(&ctx, &opts.state, &node_name).await?; - - // Check if relay exists - node.ask_and_get_reply::<_, RelayInfo>( - &ctx, - Request::get(format!("/node/forwarder/{relay_name}")), - ) - .await? - .found() - .into_diagnostic()? - .ok_or(miette!("Relay with name '{}' does not exist", relay_name))?; - - // Proceed with the deletion - if opts - .terminal - .confirmed_with_flag_or_prompt(cmd.yes, "Are you sure you want to delete this relay?")? - { - node.tell( - &ctx, - Request::delete(format!("/node/forwarder/{relay_name}",)), - ) - .await?; - - opts.terminal + DeleteTui::run(ctx, opts, cmd).await +} + +struct DeleteTui { + ctx: Context, + opts: CommandGlobalOpts, + node: BackgroundNode, + cmd: DeleteCommand, +} + +impl DeleteTui { + pub async fn run( + ctx: Context, + opts: CommandGlobalOpts, + cmd: DeleteCommand, + ) -> miette::Result<()> { + let node_name = { + let name = get_node_name(&opts.state, &cmd.at); + parse_node_name(&name)? + }; + let node = BackgroundNode::create(&ctx, &opts.state, &node_name).await?; + let tui = Self { + ctx, + opts, + node, + cmd, + }; + tui.delete().await + } +} + +#[ockam_core::async_trait] +impl DeleteCommandTui for DeleteTui { + const ITEM_NAME: &'static str = "relay"; + + fn cmd_arg_item_name(&self) -> Option<&str> { + self.cmd.relay_name.as_deref() + } + + fn cmd_arg_delete_all(&self) -> bool { + false + } + + fn cmd_arg_confirm_deletion(&self) -> bool { + self.cmd.yes + } + + fn terminal(&self) -> Terminal> { + self.opts.terminal.clone() + } + + async fn get_arg_item_name_or_default(&self) -> miette::Result { + self.cmd + .relay_name + .clone() + .ok_or(miette!("No relay name provided")) + } + + async fn list_items_names(&self) -> miette::Result> { + let relays: Vec = self + .node + .ask(&self.ctx, Request::get("/node/forwarder")) + .await?; + let names = relays + .into_iter() + .map(|i| i.remote_address().to_string()) + .collect(); + Ok(names) + } + + async fn delete_single(&self, item_name: &str) -> miette::Result<()> { + let node_name = self.node.node_name(); + self.node + .tell( + &self.ctx, + Request::delete(format!("/node/forwarder/{item_name}")), + ) + .await?; + self.terminal() .stdout() .plain(fmt_ok!( - "Relay with name {} on Node {} has been deleted.", - relay_name, - node_name + "Relay with name {} on Node {} has been deleted", + item_name.light_magenta(), + node_name.light_magenta() )) - .machine(&relay_name) - .json(serde_json::json!({ "name": relay_name, "node": node_name })) - .write_line() - .unwrap(); + .write_line()?; + Ok(()) + } + + async fn delete_multiple(&self, items_names: Vec) -> miette::Result<()> { + let node_name = self.node.node_name(); + let mut plain = String::new(); + for item_name in items_names { + let res = self + .node + .tell( + &self.ctx, + Request::delete(format!("/node/forwarder/{item_name}")), + ) + .await; + if res.is_ok() { + plain.push_str(&fmt_ok!( + "Relay with name {} on Node {} has been deleted\n", + item_name.light_magenta(), + node_name.light_magenta() + )); + } else { + plain.push_str(&fmt_warn!( + "Failed to delete relay with name {} on Node {}\n", + item_name.light_magenta(), + node_name.light_magenta() + )); + } + } + self.terminal().stdout().plain(plain).write_line()?; + Ok(()) } - Ok(()) } diff --git a/implementations/rust/ockam/ockam_command/src/relay/mod.rs b/implementations/rust/ockam/ockam_command/src/relay/mod.rs index 2ac0d761284..5e619126b03 100644 --- a/implementations/rust/ockam/ockam_command/src/relay/mod.rs +++ b/implementations/rust/ockam/ockam_command/src/relay/mod.rs @@ -11,6 +11,7 @@ mod create; mod delete; mod list; mod show; +mod util; const LONG_ABOUT: &str = include_str!("./static/long_about.txt"); const AFTER_LONG_HELP: &str = include_str!("./static/after_long_help.txt"); diff --git a/implementations/rust/ockam/ockam_command/src/relay/show.rs b/implementations/rust/ockam/ockam_command/src/relay/show.rs index f99f387f4be..021435d9626 100644 --- a/implementations/rust/ockam/ockam_command/src/relay/show.rs +++ b/implementations/rust/ockam/ockam_command/src/relay/show.rs @@ -1,9 +1,9 @@ use clap::Args; +use console::Term; use indoc::formatdoc; -use miette::IntoDiagnostic; +use miette::{miette, IntoDiagnostic}; use ockam::Context; -use ockam_api::address::extract_address_value; use ockam_api::nodes::models::relay::RelayInfo; use ockam_api::nodes::BackgroundNode; use ockam_core::api::Request; @@ -13,26 +13,27 @@ use serde::Serialize; use crate::node::get_node_name; use crate::output::Output; -use crate::util::node_rpc; -use crate::{docs, CommandGlobalOpts}; +use crate::relay::util::relay_name_parser; +use crate::terminal::tui::ShowCommandTui; +use crate::util::{node_rpc, parse_node_name}; +use crate::{docs, CommandGlobalOpts, Terminal, TerminalStream}; const PREVIEW_TAG: &str = include_str!("../static/preview_tag.txt"); const AFTER_LONG_HELP: &str = include_str!("./static/show/after_long_help.txt"); -/// Show a Relay by its alias +/// Show a Relay given its name #[derive(Clone, Debug, Args)] #[command( - arg_required_else_help = false, before_help = docs::before_help(PREVIEW_TAG), after_long_help = docs::after_help(AFTER_LONG_HELP) )] pub struct ShowCommand { - /// Name assigned to relay that will be shown (prefixed with forward_to_) - #[arg(display_order = 900, required = true)] - remote_address: String, + /// Name assigned to the Relay, prefixed with 'forward_to_'. Example: 'forward_to_myrelay' + #[arg(value_parser = relay_name_parser)] + relay_name: Option, - /// Node which relay belongs to - #[arg(display_order = 901, global = true, long, value_name = "NODE")] + /// Node which the relay belongs to + #[arg(long, value_name = "NODE")] pub at: Option, } @@ -42,6 +43,116 @@ impl ShowCommand { } } +async fn run_impl( + ctx: Context, + (opts, cmd): (CommandGlobalOpts, ShowCommand), +) -> miette::Result<()> { + ShowTui::run(ctx, opts, cmd).await +} + +pub struct ShowTui { + ctx: Context, + opts: CommandGlobalOpts, + node: BackgroundNode, + cmd: ShowCommand, +} + +impl ShowTui { + pub async fn run( + ctx: Context, + opts: CommandGlobalOpts, + cmd: ShowCommand, + ) -> miette::Result<()> { + let node_name = { + let name = get_node_name(&opts.state, &cmd.at); + parse_node_name(&name)? + }; + let node = BackgroundNode::create(&ctx, &opts.state, &node_name).await?; + let tui = Self { + ctx, + opts, + node, + cmd, + }; + tui.show().await + } +} + +#[ockam_core::async_trait] +impl ShowCommandTui for ShowTui { + const ITEM_NAME: &'static str = "nodes"; + + fn cmd_arg_item_name(&self) -> Option<&str> { + self.cmd.relay_name.as_deref() + } + + fn terminal(&self) -> Terminal> { + self.opts.terminal.clone() + } + + async fn get_arg_item_name_or_default(&self) -> miette::Result { + self.cmd + .relay_name + .clone() + .ok_or(miette!("No relay name provided")) + } + + async fn list_items_names(&self) -> miette::Result> { + let relays: Vec = self + .node + .ask(&self.ctx, Request::get("/node/forwarder")) + .await?; + let names = relays + .into_iter() + .map(|i| i.remote_address().to_string()) + .collect(); + Ok(names) + } + + async fn show_single(&self, item_name: &str) -> miette::Result<()> { + let relay: RelayInfo = self + .node + .ask( + &self.ctx, + Request::get(format!("/node/forwarder/{item_name}")), + ) + .await?; + let relay = RelayShowOutput::from(relay); + self.terminal() + .stdout() + .plain(relay.output()?) + .machine(item_name) + .json(serde_json::to_string(&relay).into_diagnostic()?) + .write_line()?; + Ok(()) + } + + async fn show_multiple(&self, items_names: Vec) -> miette::Result<()> { + let relays: Vec = self + .node + .ask(&self.ctx, Request::get("/node/forwarder")) + .await?; + let relays = relays + .into_iter() + .filter(|it| items_names.contains(&it.remote_address().to_string())) + .map(RelayShowOutput::from) + .collect::>(); + let node_name = self.node.node_name(); + let plain = self.terminal().build_list( + &relays, + &format!("Relays on Node {node_name}"), + &format!("No Relays found on Node {node_name}."), + )?; + let json = serde_json::to_string(&relays).into_diagnostic()?; + self.terminal() + .stdout() + .plain(plain) + .json(json) + .write_line()?; + Ok(()) + } +} + #[derive(Serialize)] struct RelayShowOutput { pub relay_route: String, @@ -49,6 +160,16 @@ struct RelayShowOutput { pub worker_address: MultiAddr, } +impl From for RelayShowOutput { + fn from(r: RelayInfo) -> Self { + Self { + relay_route: r.forwarding_route().to_string(), + remote_address: r.remote_address_ma().into_diagnostic().unwrap(), + worker_address: r.worker_address_ma().into_diagnostic().unwrap(), + } + } +} + impl Output for RelayShowOutput { fn output(&self) -> crate::error::Result { Ok(formatdoc!( @@ -63,34 +184,16 @@ impl Output for RelayShowOutput { worker_addr = self.worker_address, )) } -} -async fn run_impl( - ctx: Context, - (opts, cmd): (CommandGlobalOpts, ShowCommand), -) -> miette::Result<()> { - let at = get_node_name(&opts.state, &cmd.at); - let node_name = extract_address_value(&at)?; - let remote_address = &cmd.remote_address; - let node = BackgroundNode::create(&ctx, &opts.state, &node_name).await?; - let relay_info: RelayInfo = node - .ask( - &ctx, - Request::get(format!("/node/forwarder/{remote_address}")), - ) - .await?; - - let output = RelayShowOutput { - relay_route: relay_info.forwarding_route().to_string(), - remote_address: relay_info.remote_address_ma().into_diagnostic()?, - worker_address: relay_info.worker_address_ma().into_diagnostic()?, - }; - - opts.terminal - .stdout() - .plain(output.output()?) - .json(serde_json::to_string(&output).into_diagnostic()?) - .write_line()?; - - Ok(()) + fn list_output(&self) -> crate::error::Result { + Ok(formatdoc!( + r#" + Relay Route: {route} + Remote Address: {remote_addr} + Worker Address: {worker_addr}"#, + route = self.relay_route, + remote_addr = self.remote_address, + worker_addr = self.worker_address, + )) + } } diff --git a/implementations/rust/ockam/ockam_command/src/relay/util.rs b/implementations/rust/ockam/ockam_command/src/relay/util.rs new file mode 100644 index 00000000000..a84afb2ded5 --- /dev/null +++ b/implementations/rust/ockam/ockam_command/src/relay/util.rs @@ -0,0 +1,10 @@ +use crate::Result; +use miette::miette; + +pub fn relay_name_parser(arg: &str) -> Result { + if arg.starts_with("forward_to_") { + Ok(arg.to_string()) + } else { + Err(miette!("The relay name must be prefixed with 'forward_to_'").into()) + } +} diff --git a/implementations/rust/ockam/ockam_command/src/tcp/inlet/delete.rs b/implementations/rust/ockam/ockam_command/src/tcp/inlet/delete.rs index fb23fa31cce..def59da1cb4 100644 --- a/implementations/rust/ockam/ockam_command/src/tcp/inlet/delete.rs +++ b/implementations/rust/ockam/ockam_command/src/tcp/inlet/delete.rs @@ -111,7 +111,7 @@ impl DeleteCommandTui for DeleteTui { } async fn delete_single(&self, item_name: &str) -> miette::Result<()> { - let node_name = self.node.name(); + let node_name = self.node.node_name(); self.node.delete_inlet(&self.ctx, item_name).await?; self.terminal() .stdout() @@ -125,7 +125,7 @@ impl DeleteCommandTui for DeleteTui { } async fn delete_multiple(&self, items_names: Vec) -> miette::Result<()> { - let node_name = self.node.name(); + let node_name = self.node.node_name(); let mut plain = String::new(); for item_name in items_names { if self.node.delete_inlet(&self.ctx, &item_name).await.is_ok() { diff --git a/implementations/rust/ockam/ockam_command/tests/bats/relay.bats b/implementations/rust/ockam/ockam_command/tests/bats/relay.bats index 2ec4dc3c2a4..a018faabd13 100644 --- a/implementations/rust/ockam/ockam_command/tests/bats/relay.bats +++ b/implementations/rust/ockam/ockam_command/tests/bats/relay.bats @@ -38,7 +38,7 @@ teardown() { run_success $OCKAM relay create blue --at /node/n1 --to /node/n2 run_success $OCKAM relay create red --at /node/n1 --to /node/n2 - run_success $OCKAM relay list --to /node/n2 + run_success $OCKAM relay list --to /node/n2 --output json assert_output --partial "\"remote_address\": \"forward_to_blue\"" assert_output --partial "\"remote_address\": \"forward_to_red\"" @@ -47,17 +47,33 @@ teardown() { assert_output --partial "[]" } -@test "relay - show a relay on a node" { +@test "relay - CRUD" { run_success --separate-stderr "$OCKAM" node create n1 run_success --separate-stderr "$OCKAM" node create n2 - run_success "$OCKAM" relay create blue --at /node/n1 --to /node/n2 - run_success "$OCKAM" relay show forward_to_blue --at /node/n2 + # Create and show it + run_success "$OCKAM" relay create blue --at /node/n1 --to /node/n2 + run_success "$OCKAM" relay show forward_to_blue --at /node/n2 --output json assert_output --regexp "\"relay_route\".* => 0#forward_to_blue" assert_output --partial "\"remote_address\":\"/service/forward_to_blue\"" assert_output --regexp "\"worker_address\":\"/service/.*" - # Test showing non-existing with no relay - run_failure "$OCKAM" relay show relay_blank --at /node/n2 + ## Try to show a non-existing relay + run_failure "$OCKAM" relay show forward_to_r --at /node/n2 assert_output --partial "not found" + + # Create another one and list both + run_success "$OCKAM" relay create red --at /node/n1 --to /node/n2 + run_success "$OCKAM" relay list --to /node/n2 --output json + assert_output --partial "\"remote_address\": \"forward_to_blue\"" + assert_output --partial "\"remote_address\": \"forward_to_red\"" + + # Delete the first + run_success "$OCKAM" relay delete -y forward_to_blue --at /node/n2 + run_success "$OCKAM" relay list --to /node/n2 --output json + refute_output --partial "\"remote_address\": \"forward_to_blue\"" + assert_output --partial "\"remote_address\": \"forward_to_red\"" + + ## Try to delete twice + run_failure "$OCKAM" relay delete -y forward_to_blue --at /node/n2 }