diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 8f056999..33fe0400 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -596,9 +596,9 @@ checksum = "c8d4a3bb8b1e0c1050499d1815f5ab16d04f0959b233085fb31653fbfc9d98f9" [[package]] name = "cli-engine" -version = "0.10.2" +version = "0.10.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d4efc6aa4b77c5a0e328f715238a85b91c98e4eea8bf19083ed0466459ecfa6" +checksum = "4b050b54bc79c2a5cc65e3df416f494a0cd30ea407e87a4a6cc7830c4ce4c397" dependencies = [ "async-trait", "base64", diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 28e84d08..eb5b06ef 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -28,8 +28,8 @@ chrono = { version = "0.4", default-features = false, features = ["clock"] } async-trait = "0.1" bytes = "1" chrono = { version = "0.4", default-features = false, features = ["clock", "serde"] } -clap = { version = "4.5", features = ["std", "string", "derive"] } -cli-engine = { version = "0.10.2", features = ["pkce-auth"] } +clap = { version = "4.5", features = ["std", "string", "derive", "env"] } +cli-engine = { version = "0.10.3", features = ["pkce-auth"] } dirs = "6" domains-client = { path = "domains-client" } email-client = { path = "email-client" } @@ -53,7 +53,7 @@ phonenumber = "0.3" progenitor-client = "0.14" regex = { version = "1", features = ["std"] } reqwest = { version = "0.13", default-features = false, features = ["json", "multipart", "form", "rustls"] } -schemars = { version = "1.2", features = ["derive"], optional = true } +schemars = { version = "1.2", features = ["derive"] } self-replace = "1.5.0" semver = { version = "1.0.28", default-features = false, features = ["std"] } sha2 = "0.10" @@ -79,7 +79,7 @@ tempfile = "3" [features] # Excludes `gddy spec` from default/release builds: it's only used to extract # the CLI's command spec for the dev-portal, never by end users. -dev-portal-spec = ["dep:schemars"] +dev-portal-spec = [] [lints.rust] unsafe_code = "deny" diff --git a/rust/src/dns/add.rs b/rust/src/dns/add.rs index 13d63ec0..d5da2c9b 100644 --- a/rust/src/dns/add.rs +++ b/rust/src/dns/add.rs @@ -124,7 +124,7 @@ pub(super) fn command() -> RuntimeCommandSpec { let domain = args.domain; let record_type = args.record_type; let name = args.name; - let domain_format = args.domain_format.form(); + let domain_format = args.domain_format.form(&ctx)?; validate_caa_fields(&record_type, &opts) .map_err(crate::error::GddyError::validation)?; validate_tlsa_fields(&record_type, &opts) diff --git a/rust/src/dns/delete.rs b/rust/src/dns/delete.rs index d6af8937..12dfb948 100644 --- a/rust/src/dns/delete.rs +++ b/rust/src/dns/delete.rs @@ -176,7 +176,7 @@ pub(super) fn command() -> RuntimeCommandSpec { let domain = args.domain; let record_type = args.record_type; let name = args.name; - let domain_format = args.domain_format.form(); + let domain_format = args.domain_format.form(&ctx)?; let debug = !ctx.middleware.debug.is_empty(); let client = make_client(&ctx).await?; diff --git a/rust/src/dns/guides/dns.md b/rust/src/dns/guides/dns.md index 9ef96c19..bb29917a 100644 --- a/rust/src/dns/guides/dns.md +++ b/rust/src/dns/guides/dns.md @@ -67,7 +67,7 @@ Deleting something that doesn't match is a no-op success (`deleted: 0`), not an ## International domain names -Domain names and record names can be typed either as Unicode (`münchen.de`) or in their encoded form (`xn--mnchen-3ya.de`); both reach the same domain. Output shows Unicode by default. Add `--punycode` to any DNS command to show the encoded form instead. Record values (the `--data` of a CNAME, MX or TXT record) are never converted; enter them exactly as they should be stored. +Domain names and record names can be typed either as Unicode (`münchen.de`) or in their encoded form (`xn--mnchen-3ya.de`); both reach the same domain. Output shows Unicode by default. Add `--punycode` to any DNS command to show the encoded form instead, or make that your default with `gddy config set punycode true` (undo it with `gddy config unset punycode`). Precedence is the `--punycode` flag, then the `GDDY_PUNYCODE` environment variable, then your saved setting, then Unicode, so `--punycode=false` turns the encoded form back off for one command. `gddy config list` shows the current value. Record values (the `--data` of a CNAME, MX or TXT record) are never converted; enter them exactly as they should be stored. ## Type-specific flags diff --git a/rust/src/dns/list.rs b/rust/src/dns/list.rs index d370785f..a785cba1 100644 --- a/rust/src/dns/list.rs +++ b/rust/src/dns/list.rs @@ -92,8 +92,9 @@ pub(super) fn command() -> RuntimeCommandSpec { let mut out = build_list_output(&records).map_err(|e| { CliCoreError::message(format!("failed to serialize DNS records: {e}")) })?; + let domain_format = args.domain_format.form(&ctx)?; for record in &mut out { - args.domain_format.form().convert_fields(record, &["name"]); + domain_format.convert_fields(record, &["name"]); } Ok(CommandResult::new(json!(out))) }, diff --git a/rust/src/dns/set/mod.rs b/rust/src/dns/set/mod.rs index fd4fe040..9ce40b35 100644 --- a/rust/src/dns/set/mod.rs +++ b/rust/src/dns/set/mod.rs @@ -99,7 +99,7 @@ pub(super) fn command() -> RuntimeCommandSpec { let domain = args.write.domain; let record_type = args.write.record_type; let name = args.write.name; - let domain_format = args.write.domain_format.form(); + let domain_format = args.write.domain_format.form(&ctx)?; let replace_conflicting = args.replace_conflicting_types; validate_caa_fields(&record_type, &opts) .map_err(crate::error::GddyError::validation)?; diff --git a/rust/src/domain/auto_renew.rs b/rust/src/domain/auto_renew.rs index dafb1dfd..7cc73a71 100644 --- a/rust/src/domain/auto_renew.rs +++ b/rust/src/domain/auto_renew.rs @@ -9,6 +9,7 @@ use serde_json::json; use super::common::{ api_error, format_operation_error, is_terminal_status, make_client, validate_domain_name, }; +use crate::idn::DomainFormArgs; use crate::next_action::next_action; use crate::output_schema::output_schema; use crate::scopes::DOMAINS_DOMAIN_UPDATE; @@ -42,6 +43,9 @@ struct AutoRenewSetArgs { #[arg(value_name = "DOMAIN")] domain: String, + #[command(flatten)] + domain_format: DomainFormArgs, + #[command(flatten)] toggle: AutoRenewToggle, } @@ -77,6 +81,9 @@ pub(super) fn group() -> RuntimeGroupSpec { // Clap's ArgGroup guarantees exactly one of --enable/--disable was passed. let enabled = args.toggle.enable; let debug = !ctx.middleware.debug.is_empty(); + // Resolve the display form before the update is sent, so a bad + // preference can't hide the operation ID of a change already made. + let domain_format = args.domain_format.form(&ctx)?; let client = make_client(&ctx).await?; let idempotency_key = uuid::Uuid::new_v4().to_string(); @@ -120,6 +127,8 @@ pub(super) fn group() -> RuntimeGroupSpec { result["status"] = json!(status); } + domain_format.convert_fields(&mut result, &["domain"]); + let mut actions = Vec::new(); if is_terminal_status(&status) { actions.push( @@ -127,7 +136,10 @@ pub(super) fn group() -> RuntimeGroupSpec { "domain get ", "Confirm the updated auto-renew setting", ) - .with_param("domain", NextActionParam::value(domain)), + .with_param( + "domain", + NextActionParam::value(domain_format.render(&domain)), + ), ); } else if let Some(op_id) = operation_id.as_ref() { actions.push( diff --git a/rust/src/domain/available.rs b/rust/src/domain/available.rs index b1086798..ce52f926 100644 --- a/rust/src/domain/available.rs +++ b/rust/src/domain/available.rs @@ -182,7 +182,7 @@ pub(super) fn command() -> RuntimeCommandSpec { } } - let domain_format = args.domain_format.form(); + let domain_format = args.domain_format.form(&ctx)?; domain_format.convert_fields(&mut result, &["domain"]); let resolved_domain = domain_format.render(&resolved_domain); diff --git a/rust/src/domain/get.rs b/rust/src/domain/get.rs index 31b8b9fb..737609f5 100644 --- a/rust/src/domain/get.rs +++ b/rust/src/domain/get.rs @@ -50,7 +50,7 @@ pub(super) fn command() -> RuntimeCommandSpec { CliCoreError::message(format!("failed to serialize domain details: {e}")) })?; args.domain_format - .form() + .form(&ctx)? .convert_fields(&mut value, &["domain", "nameServers"]); Ok(CommandResult::new(value).with_next_actions(vec![ next_action("dns list ", "View this domain's DNS records") diff --git a/rust/src/domain/guides/domain-purchase.md b/rust/src/domain/guides/domain-purchase.md index 1b14db79..10ebbbc9 100644 --- a/rust/src/domain/guides/domain-purchase.md +++ b/rust/src/domain/guides/domain-purchase.md @@ -67,6 +67,10 @@ token has expired or isn't found, just re-run `gddy domain quote`. AI assistants: Before using `--agree` or `--confirm`, show the end user the exact price from `domain quote` (including any premium/registration fees) and each required legal agreement's title AND link (from `quote`'s `requiredAgreements`), and obtain their explicit confirmation to proceed. Do not summarize an agreement by name only — the user must be able to click through and read it. Do not infer confirmation from a general request to buy or register a domain. +## International domain names + +You can type a name either as Unicode (`münchen.de`) or in its encoded form (`xn--mnchen-3ya.de`); both reach the same domain. Output shows Unicode by default. Add `--punycode` to any `domain` command that shows names to see the encoded form instead, or make that your default with `gddy config set punycode true` (undo it with `gddy config unset punycode`). Precedence is the `--punycode` flag, then the `GDDY_PUNYCODE` environment variable, then your saved setting, then Unicode, so `--punycode=false` turns the encoded form back off for one command. + ## Contacts A registration has four contact roles: **registrant**, **admin**, **billing**, diff --git a/rust/src/domain/list.rs b/rust/src/domain/list.rs index 957fc854..7a06e366 100644 --- a/rust/src/domain/list.rs +++ b/rust/src/domain/list.rs @@ -280,10 +280,9 @@ pub(super) fn command() -> RuntimeCommandSpec { .map_err(|e| { CliCoreError::message(format!("failed to serialize domain list: {e}")) })?; + let domain_format = args.domain_format.form(&ctx)?; for domain in &mut domains { - args.domain_format - .form() - .convert_fields(domain, &["domain", "nameServers"]); + domain_format.convert_fields(domain, &["domain", "nameServers"]); } Ok(CommandResult::new(json!(domains)).with_next_actions(vec![ next_action("dns list ", "View a domain's DNS records") diff --git a/rust/src/domain/nameservers.rs b/rust/src/domain/nameservers.rs index ca100d6d..6ccbd4d2 100644 --- a/rust/src/domain/nameservers.rs +++ b/rust/src/domain/nameservers.rs @@ -61,6 +61,9 @@ pub(super) fn group() -> RuntimeGroupSpec { let domain = validate_domain_name(&args.domain)?; let hosts = validate_nameserver_hosts(args.nameserver)?; let debug = !ctx.middleware.debug.is_empty(); + // Resolve the display form before the update is sent, so a bad + // preference can't hide the operation ID of a change already made. + let domain_format = args.domain_format.form(&ctx)?; let client = make_client(&ctx).await?; let body = types::NameServers( @@ -93,7 +96,6 @@ pub(super) fn group() -> RuntimeGroupSpec { if let Some(status) = op.status.as_ref() { result["status"] = json!(status.to_string()); } - let domain_format = args.domain_format.form(); domain_format.convert_fields(&mut result, &["domain", "nameservers"]); Ok(CommandResult::new(result).with_next_actions(vec![ next_action("domain get ", "Confirm the updated nameservers").with_param( diff --git a/rust/src/domain/operation.rs b/rust/src/domain/operation.rs index f21fdd41..1c5518af 100644 --- a/rust/src/domain/operation.rs +++ b/rust/src/domain/operation.rs @@ -85,7 +85,7 @@ fn status_command() -> RuntimeCommandSpec { .unwrap_or(operation_id); let op_type = op.type_.as_ref().map(|t| t.to_string()).unwrap_or_default(); - let domain_format = args.domain_format.form(); + let domain_format = args.domain_format.form(&ctx)?; let domain = domain_format.render(&domain); let mut result = json!({ "operationId": op_id, diff --git a/rust/src/domain/purchase.rs b/rust/src/domain/purchase.rs index c018ee82..4f771775 100644 --- a/rust/src/domain/purchase.rs +++ b/rust/src/domain/purchase.rs @@ -175,6 +175,9 @@ pub(super) fn command() -> RuntimeCommandSpec { let agree = args.agree; let confirm = args.confirm; let debug = !ctx.middleware.debug.is_empty(); + // Resolve the display form up front: a bad preference must fail + // before the registration is submitted, not after the charge. + let domain_format = args.domain_format.form(&ctx)?; // Resolve auth *before* consuming the cached quote, so an auth // failure fails locally rather than after the cache entry (and the @@ -406,9 +409,7 @@ pub(super) fn command() -> RuntimeCommandSpec { ); } } - args.domain_format - .form() - .convert_fields(&mut result, &["domain"]); + domain_format.convert_fields(&mut result, &["domain"]); Ok(CommandResult::new(result).with_next_actions(actions)) }, ) diff --git a/rust/src/domain/quote.rs b/rust/src/domain/quote.rs index f876f76d..78c7ac02 100644 --- a/rust/src/domain/quote.rs +++ b/rust/src/domain/quote.rs @@ -323,6 +323,9 @@ pub(super) fn command() -> RuntimeCommandSpec { // build the profile. The token is bound to a hash of this quoted // request, so what's quoted here is what `purchase` later registers. let client = make_client(&ctx).await?; + // Resolve the display form before a quote is requested and cached, + // so a bad preference can't discard a token that was just issued. + let domain_format = args.domain_format.form(&ctx)?; let profile = build_profile(privacy, renew_auto, &name_servers)?; // Cache the exact profile we quote with: the token binds a hash of the // domain/price/profile, so `purchase` must re-send this verbatim or the @@ -410,17 +413,15 @@ pub(super) fn command() -> RuntimeCommandSpec { // same next step `domain available` offers for a taken name. // `domain suggest` accepts a seed domain, so the domain just // quoted is a valid query to copy/paste directly. - let resolved_domain = args - .domain_format - .form() - .render("e.domain.clone().unwrap_or_else(|| domain.clone())); + let resolved_domain = + domain_format.render("e.domain.clone().unwrap_or_else(|| domain.clone())); next_actions.push( next_action("domain suggest ", "Find an available alternative") .with_param("query", NextActionParam::value(resolved_domain)), ); } - args.domain_format.form().convert_fields(&mut view, &["domain", "resolved.nameServers"]); + domain_format.convert_fields(&mut view, &["domain", "resolved.nameServers"]); Ok(CommandResult::new(view).with_next_actions(next_actions)) }, ) diff --git a/rust/src/domain/suggest.rs b/rust/src/domain/suggest.rs index e56fa0e4..a11f6b8c 100644 --- a/rust/src/domain/suggest.rs +++ b/rust/src/domain/suggest.rs @@ -209,7 +209,7 @@ pub(super) fn command() -> RuntimeCommandSpec { Ok(r) => r.into_inner(), Err(e) => return Err(api_error("domain suggestion", debug, e).await), }; - let domain_format = args.domain_format.form(); + let domain_format = args.domain_format.form(&ctx)?; let suggestions: Vec = resp .items .iter() diff --git a/rust/src/domain/transfer_lock.rs b/rust/src/domain/transfer_lock.rs index 970e83d8..e9ea9468 100644 --- a/rust/src/domain/transfer_lock.rs +++ b/rust/src/domain/transfer_lock.rs @@ -9,6 +9,7 @@ use serde_json::json; use super::common::{ api_error, format_operation_error, is_terminal_status, make_client, validate_domain_name, }; +use crate::idn::DomainFormArgs; use crate::next_action::next_action; use crate::output_schema::output_schema; use crate::scopes::DOMAINS_DOMAIN_UPDATE; @@ -43,6 +44,9 @@ struct TransferLockSetArgs { #[arg(value_name = "DOMAIN")] domain: String, + #[command(flatten)] + domain_format: DomainFormArgs, + #[command(flatten)] toggle: TransferLockToggle, } @@ -81,6 +85,9 @@ pub(super) fn group() -> RuntimeGroupSpec { // Clap's ArgGroup guarantees exactly one of --enable/--disable was passed. let enabled = args.toggle.enable; let debug = !ctx.middleware.debug.is_empty(); + // Resolve the display form before the update is sent, so a bad + // preference can't hide the operation ID of a change already made. + let domain_format = args.domain_format.form(&ctx)?; let client = make_client(&ctx).await?; let idempotency_key = uuid::Uuid::new_v4().to_string(); @@ -124,6 +131,8 @@ pub(super) fn group() -> RuntimeGroupSpec { result["status"] = json!(status); } + domain_format.convert_fields(&mut result, &["domain"]); + let mut actions = Vec::new(); if is_terminal_status(&status) { actions.push( @@ -131,7 +140,10 @@ pub(super) fn group() -> RuntimeGroupSpec { "domain get ", "Confirm the updated transfer-lock setting", ) - .with_param("domain", NextActionParam::value(domain)), + .with_param( + "domain", + NextActionParam::value(domain_format.render(&domain)), + ), ); } else if let Some(op_id) = operation_id.as_ref() { actions.push( diff --git a/rust/src/environments/test_support.rs b/rust/src/environments/test_support.rs index 61c85299..b50ccdde 100644 --- a/rust/src/environments/test_support.rs +++ b/rust/src/environments/test_support.rs @@ -26,13 +26,25 @@ pub(crate) static ENV_LOCK: Mutex<()> = Mutex::const_new(()); /// value back if it was — even if a test panics. Restoring rather than /// unconditionally removing keeps a var a developer happens to already /// have set in their shell from leaking into the rest of the test run. -pub(super) struct EnvGuard { +pub(crate) struct EnvGuard { key: &'static str, prior: Option, } impl EnvGuard { - pub(super) fn set(key: &'static str, value: &str) -> Self { + /// Removes `key` for the guard's lifetime, so a developer's own shell + /// value can't leak into the test. + pub(crate) fn unset(key: &'static str) -> Self { + let prior = std::env::var(key).ok(); + // SAFETY: caller holds ENV_LOCK. + #[allow(unsafe_code)] + unsafe { + std::env::remove_var(key); + } + Self { key, prior } + } + + pub(crate) fn set(key: &'static str, value: &str) -> Self { let prior = std::env::var(key).ok(); // SAFETY: caller holds ENV_LOCK. #[allow(unsafe_code)] diff --git a/rust/src/hosting/domain/attach.rs b/rust/src/hosting/domain/attach.rs index 73e621c4..cabbaeac 100644 --- a/rust/src/hosting/domain/attach.rs +++ b/rust/src/hosting/domain/attach.rs @@ -36,14 +36,15 @@ pub(super) fn command() -> RuntimeCommandSpec { .with_output_schema::(), |ctx, args: DomainAttachArgs| async move { let app_id = args.app_id.clone(); + // Resolve the display form before attaching, so a bad preference + // can't hide the details of an attachment that already succeeded. + let domain_format = args.domain_format.form(&ctx)?; let client = make_client(&ctx, &[DOMAIN_WRITE]).await?; let mut data = client .attach_domain(&app_id, &args.hostname) .await .map_err(client_err)?; - args.domain_format - .form() - .convert_fields(&mut data, &["hostname"]); + domain_format.convert_fields(&mut data, &["hostname"]); Ok(CommandResult::new(data).with_next_actions(vec![ next_action( "hosting domain list --app-id ", diff --git a/rust/src/hosting/domain/get.rs b/rust/src/hosting/domain/get.rs index 99494073..97d627b7 100644 --- a/rust/src/hosting/domain/get.rs +++ b/rust/src/hosting/domain/get.rs @@ -40,7 +40,7 @@ pub(super) fn command() -> RuntimeCommandSpec { .await .map_err(client_err)?; args.domain_format - .form() + .form(&ctx)? .convert_fields(&mut data, &["hostname"]); Ok(CommandResult::new(data).with_next_actions(vec![ next_action( diff --git a/rust/src/hosting/domain/list.rs b/rust/src/hosting/domain/list.rs index 268944e1..9844a956 100644 --- a/rust/src/hosting/domain/list.rs +++ b/rust/src/hosting/domain/list.rs @@ -64,10 +64,9 @@ pub(super) fn command() -> RuntimeCommandSpec { } } + let domain_format = args.domain_format.form(&ctx)?; for item in &mut all_items { - args.domain_format - .form() - .convert_fields(item, &["hostname"]); + domain_format.convert_fields(item, &["hostname"]); } Ok(CommandResult::new(json!(all_items)).with_next_actions(vec![ next_action( diff --git a/rust/src/idn.rs b/rust/src/idn.rs index eeec09b4..9d87d0cb 100644 --- a/rust/src/idn.rs +++ b/rust/src/idn.rs @@ -4,11 +4,13 @@ //! the API ([`to_ascii`]). Names shown to the user are Unicode by default and //! ASCII when the user asks for it ([`DomainForm`]). +use cli_engine::CommandContext; use serde_json::Value; use cli_engine::DetailedError; use crate::error::GddyError; +use crate::prefs::Prefs; /// Prefix of an ASCII-compatible-encoded (punycode) label, any case. const ACE_PREFIX: &str = "xn--"; @@ -111,16 +113,35 @@ impl DomainForm { } /// `--punycode`, shared by every command that shows domain names. +/// +/// Precedence: the flag, then `GDDY_PUNYCODE`, then the saved `punycode` +/// preference, then Unicode. #[derive(Debug, Clone, Copy, Default, clap::Args)] pub(crate) struct DomainFormArgs { - /// Show domain names in their ASCII (punycode) form, e.g. xn--mnchen-3ya.de, instead of Unicode. - #[arg(long)] - punycode: bool, + /// Show domain names in their ASCII (punycode) form, e.g. xn--mnchen-3ya.de, instead of + /// Unicode. Use `--punycode=false` to override a saved preference. + #[arg( + long, + env = "GDDY_PUNYCODE", + value_name = "BOOL", + num_args = 0..=1, + require_equals = true, + default_missing_value = "true", + value_parser = clap::builder::BoolishValueParser::new(), + )] + punycode: Option, } impl DomainFormArgs { - pub(crate) fn form(&self) -> DomainForm { - DomainForm::from_flag(self.punycode) + /// The form to show names in. The config file is only read when neither the + /// flag nor the environment variable decided, so an explicit choice works + /// even while the file is broken. + pub(crate) fn form(&self, ctx: &CommandContext) -> Result { + let punycode = match self.punycode { + Some(explicit) => explicit, + None => Prefs::load(ctx)?.punycode, + }; + Ok(DomainForm::from_flag(punycode)) } } @@ -260,4 +281,162 @@ mod tests { DomainForm::Unicode.convert_fields(&mut v, &["domain", "nameServers.a.b", "items.x"]); assert_eq!(v, before); } + + mod precedence { + use cli_engine::{ + Cli, CliConfig, CommandResult, CommandSpec, GroupSpec, Module, RuntimeCommandSpec, + RuntimeGroupSpec, + }; + + use serde_json::json; + + use super::super::*; + use crate::environments::test_support::{ENV_LOCK, EnvGuard}; + + #[derive(Debug, Clone, clap::Args)] + struct ShowArgs { + #[command(flatten)] + domain_format: DomainFormArgs, + } + + /// A CLI with one command reporting which form `DomainFormArgs` chose. + fn cli() -> Cli { + let module = Module::new("Test", |_ctx| { + RuntimeGroupSpec::new(GroupSpec::new("t", "test")).with_command( + RuntimeCommandSpec::new_typed_with_context::( + CommandSpec::from_args::("show", "show").no_auth(true), + |ctx, args: ShowArgs| async move { + let form = args.domain_format.form(&ctx)?; + Ok(CommandResult::new( + json!({ "punycode": form == DomainForm::Punycode }), + )) + }, + ), + ) + }); + Cli::new( + CliConfig::new("gddy", "test", "gddy") + .with_config_schema::() + .with_startup_args(Vec::<&str>::new()) + .with_module(module), + ) + } + + /// Runs `gddy t show ` with the given saved config and env var. + async fn run(config: Option<&str>, env: Option<&str>, flag: Option<&str>) -> (i32, String) { + let _lock = ENV_LOCK.lock().await; + let dir = tempfile::tempdir().expect("tempdir"); + let _xdg = EnvGuard::set("XDG_CONFIG_HOME", &dir.path().display().to_string()); + let _env = match env { + Some(value) => EnvGuard::set("GDDY_PUNYCODE", value), + None => EnvGuard::unset("GDDY_PUNYCODE"), + }; + if let Some(contents) = config { + let path = dir.path().join("gddy"); + std::fs::create_dir_all(&path).expect("mkdir"); + std::fs::write(path.join("config.toml"), contents).expect("write config"); + } + let mut args = vec!["gddy", "t", "show"]; + args.extend(flag); + let out = cli().run(args).await; + (out.exit_code, out.rendered) + } + + async fn punycode(config: Option<&str>, env: Option<&str>, flag: Option<&str>) -> bool { + let (code, rendered) = run(config, env, flag).await; + assert_eq!( + code, 0, + "config={config:?} env={env:?} flag={flag:?}: {rendered}" + ); + rendered.contains("\"punycode\": true") + } + + const ON: Option<&str> = Some("punycode = true\n"); + const OFF: Option<&str> = Some("punycode = false\n"); + + #[tokio::test] + async fn defaults_to_unicode() { + assert!(!punycode(None, None, None).await); + } + + #[tokio::test] + async fn saved_preference_applies() { + assert!(punycode(ON, None, None).await); + assert!(!punycode(OFF, None, None).await); + } + + #[tokio::test] + async fn env_beats_saved_preference() { + assert!(!punycode(ON, Some("false"), None).await); + assert!(punycode(OFF, Some("true"), None).await); + assert!(punycode(OFF, Some("1"), None).await, "boolish env values"); + } + + #[tokio::test] + async fn flag_beats_env_and_saved_preference() { + assert!(punycode(OFF, Some("false"), Some("--punycode")).await); + assert!(!punycode(ON, Some("true"), Some("--punycode=false")).await); + } + + #[tokio::test] + async fn explicit_choice_works_while_the_config_file_is_broken() { + let broken = Some("not = valid = toml"); + assert!(punycode(broken, None, Some("--punycode")).await); + assert!(punycode(broken, Some("true"), None).await); + } + + #[tokio::test] + async fn broken_config_is_an_error_not_a_silent_default() { + for broken in [Some("not = valid = toml"), Some("punycode = \"banana\"\n")] { + let (code, rendered) = run(broken, None, None).await; + assert_ne!(code, 0, "{broken:?}: {rendered}"); + assert!(rendered.contains("fix"), "{broken:?}: {rendered}"); + } + } + + /// Set only in the child process spawned by `invalid_env_value_is_rejected`. + const CHILD: &str = "GDDY_TEST_INVALID_PUNYCODE_ENV"; + + /// An invalid `GDDY_PUNYCODE` makes every command that reads it fail to + /// parse, so it must never be set in this process, where other tests + /// parse such commands without holding `ENV_LOCK`. Run it in a child + /// test process instead. + #[test] + fn invalid_env_value_is_rejected() { + let (_, module) = module_path!() + .split_once("::") + .expect("crate-qualified path"); + let status = std::process::Command::new(std::env::current_exe().expect("test binary")) + .args([ + "--exact", + &format!("{module}::invalid_env_value_is_rejected_in_child"), + ]) + .env(CHILD, "1") + .env("GDDY_PUNYCODE", "banana") + .output() + .expect("spawn test binary"); + assert!( + status.status.success(), + "child failed:\n{}\n{}", + String::from_utf8_lossy(&status.stdout), + String::from_utf8_lossy(&status.stderr) + ); + // A filter that matched nothing would also exit 0. + assert!( + String::from_utf8_lossy(&status.stdout).contains("1 passed"), + "child test did not run" + ); + } + + /// Runs only when spawned by `invalid_env_value_is_rejected`, which has + /// set the invalid value in this process's environment. + #[tokio::test] + async fn invalid_env_value_is_rejected_in_child() { + if std::env::var_os(CHILD).is_none() { + return; + } + let out = cli().run(["gddy", "t", "show"]).await; + assert_ne!(out.exit_code, 0, "{}", out.rendered); + } + } } diff --git a/rust/src/main.rs b/rust/src/main.rs index edc916d0..f8d0f354 100644 --- a/rust/src/main.rs +++ b/rust/src/main.rs @@ -17,6 +17,7 @@ mod output_schema; mod pat; mod payment_methods; mod platform; +mod prefs; mod scopes; mod shopping; #[cfg(feature = "dev-portal-spec")] @@ -83,6 +84,7 @@ async fn main() -> ExitCode { .with_date(env!("GDDY_BUILD_DATE")), ) .with_client_attribution(cli_engine::transport::AttributionConfig::new()) + .with_config_schema::() .with_default_auth_provider("godaddy") .with_auth_provider(auth_provider) .with_auth_extra_commands([scopes::auth_scopes_command()]) diff --git a/rust/src/prefs.rs b/rust/src/prefs.rs new file mode 100644 index 00000000..c02413ad --- /dev/null +++ b/rust/src/prefs.rs @@ -0,0 +1,87 @@ +//! User preferences stored in `config.toml` and managed with `gddy config`. +//! +//! Each field is a key users can `gddy config set`; doc comments are shown by +//! `gddy config list`. A preference is only a default: the matching flag and +//! `GDDY_*` environment variable override it per invocation. + +use cli_engine::CommandContext; +use schemars::JsonSchema; +use serde::{Deserialize, Deserializer, Serialize}; + +use crate::error::GddyError; + +#[derive(Debug, Default, Serialize, Deserialize, JsonSchema)] +#[serde(default)] +pub(crate) struct Prefs { + /// Show domain names in their ASCII (punycode) form, e.g. xn--mnchen-3ya.de, instead of + /// Unicode. Overridden by `--punycode` and the GDDY_PUNYCODE environment variable. + pub(crate) punycode: bool, + + /// Currency to show shopping prices in, as an ISO 4217 code such as USD or GBP. Overridden by + /// `--currency` and the GDDY_CURRENCY environment variable. + #[serde(deserialize_with = "currency_code")] + pub(crate) currency: Option, +} + +/// A saved currency must be a real ISO 4217 code, so `gddy config set currency banana` fails +/// instead of storing a value that every shopping command would then reject. +fn currency_code<'de, D: Deserializer<'de>>(deserializer: D) -> Result, D::Error> { + Option::::deserialize(deserializer)? + .map(|saved| { + crate::shopping::currency_code(&saved).map_err(|_| { + serde::de::Error::custom(format!( + "invalid currency {saved:?}: expected an ISO 4217 code such as USD or GBP" + )) + }) + }) + .transpose() +} + +impl Prefs { + /// The saved preferences. A config file that is malformed, or holds a value + /// of the wrong type, is an error rather than silently reading as defaults. + pub(crate) fn load(ctx: &CommandContext) -> Result { + ctx.config().deserialize().map_err(|e| { + GddyError::validation(e.to_string()).with_fix( + "Run `gddy config list` to inspect your settings, or `gddy config unset ` \ + to reset one.", + ) + }) + } +} + +#[cfg(test)] +mod tests { + use cli_engine::{ConfigFile, ConfigSchema}; + + use super::*; + + fn set(key: &str, raw: &str) -> Result { + let mut file = ConfigFile::default(); + ConfigSchema::of::() + .set(&mut file, key, raw) + .map(|_| file) + .map_err(|e| e.to_string()) + } + + #[test] + fn currency_accepts_iso_codes_in_any_case() { + for raw in ["USD", "gbp", "Eur"] { + let file = set("currency", raw).expect("valid currency"); + let prefs: Prefs = file.deserialize().expect("prefs"); + assert_eq!(prefs.currency.as_deref(), Some(&*raw.to_ascii_uppercase())); + } + } + + #[test] + fn currency_rejects_non_iso_values() { + let err = set("currency", "banana").expect_err("not a currency"); + assert!(err.contains("ISO 4217"), "{err}"); + } + + #[test] + fn currency_defaults_to_unset() { + let prefs: Prefs = ConfigFile::default().deserialize().expect("prefs"); + assert_eq!(prefs.currency, None); + } +} diff --git a/rust/src/shopping/catalog/get.rs b/rust/src/shopping/catalog/get.rs index e4a63fe1..ba3ce609 100644 --- a/rust/src/shopping/catalog/get.rs +++ b/rust/src/shopping/catalog/get.rs @@ -8,8 +8,9 @@ use crate::next_action::next_action; use crate::output_schema::output_schema; use crate::shopping::client::{ClientError, decode}; use crate::shopping::common::{ - client_err, currency_code, make_client, merge_context_currency, reject_response_errors, + client_err, make_client, merge_context_currency, reject_response_errors, }; +use crate::shopping::currency::CurrencyArgs; use crate::shopping::human::{CATALOG_GET_VIEW_ID, catalog_product_response}; use crate::shopping::{SHOPPING_CATALOG_SCOPES, command_for_env}; @@ -24,9 +25,8 @@ struct Args { #[arg(value_name = "ID")] id: String, - /// Preferred ISO 4217 currency for returned catalog prices (for example, USD or GBP). - #[arg(long, value_name = "CODE", value_parser = currency_code)] - currency: Option, + #[command(flatten)] + currency: CurrencyArgs, } pub(super) fn command() -> RuntimeCommandSpec { @@ -43,7 +43,8 @@ pub(super) fn command() -> RuntimeCommandSpec { id: Some(args.id), ..Default::default() }; - merge_context_currency(&mut request.context, args.currency.as_deref()); + let currency = args.currency.resolve(&ctx)?; + merge_context_currency(&mut request.context, currency.as_deref()); let client = make_client(&ctx).await?; let response = decode::( client diff --git a/rust/src/shopping/catalog/lookup.rs b/rust/src/shopping/catalog/lookup.rs index 800974bf..1e9eca28 100644 --- a/rust/src/shopping/catalog/lookup.rs +++ b/rust/src/shopping/catalog/lookup.rs @@ -5,8 +5,9 @@ use crate::output_schema::output_schema; use crate::shopping::SHOPPING_CATALOG_SCOPES; use crate::shopping::client::{ClientError, decode}; use crate::shopping::common::{ - client_err, currency_code, make_client, merge_context_currency, reject_response_errors, + client_err, make_client, merge_context_currency, reject_response_errors, }; +use crate::shopping::currency::CurrencyArgs; use crate::shopping::human::{CATALOG_LOOKUP_VIEW_ID, catalog_lookup_response}; output_schema!(CatalogLookupOutput { @@ -21,9 +22,8 @@ struct Args { #[arg(value_name = "ID", required = true)] id: Vec, - /// Preferred ISO 4217 currency for returned catalog prices (for example, USD or GBP). - #[arg(long, value_name = "CODE", value_parser = currency_code)] - currency: Option, + #[command(flatten)] + currency: CurrencyArgs, } pub(super) fn command() -> RuntimeCommandSpec { @@ -43,7 +43,8 @@ pub(super) fn command() -> RuntimeCommandSpec { ids: args.id, ..Default::default() }; - merge_context_currency(&mut request.context, args.currency.as_deref()); + let currency = args.currency.resolve(&ctx)?; + merge_context_currency(&mut request.context, currency.as_deref()); let client = make_client(&ctx).await?; let response = decode::( client diff --git a/rust/src/shopping/catalog/search.rs b/rust/src/shopping/catalog/search.rs index 3e58a178..ea4bb934 100644 --- a/rust/src/shopping/catalog/search.rs +++ b/rust/src/shopping/catalog/search.rs @@ -10,8 +10,9 @@ use crate::next_action::next_action; use crate::output_schema::output_schema; use crate::shopping::client::{ClientError, decode}; use crate::shopping::common::{ - client_err, currency_code, make_client, merge_context_currency, reject_response_errors, + client_err, make_client, merge_context_currency, reject_response_errors, }; +use crate::shopping::currency::CurrencyArgs; use crate::shopping::human::{CATALOG_SEARCH_VIEW_ID, catalog_search_response}; use crate::shopping::{SHOPPING_CATALOG_SCOPES, command_for_env}; @@ -39,9 +40,8 @@ struct Args { #[arg(long, value_name = "N", value_parser = clap::value_parser!(u8).range(1..=100))] limit: Option, - /// Preferred ISO 4217 currency for returned catalog prices (for example, USD or GBP). - #[arg(long, value_name = "CODE", value_parser = currency_code)] - currency: Option, + #[command(flatten)] + currency: CurrencyArgs, } pub(super) fn command() -> RuntimeCommandSpec { @@ -65,7 +65,8 @@ pub(super) fn command() -> RuntimeCommandSpec { &args.category, args.cursor.as_deref(), )?; - merge_context_currency(&mut request.context, args.currency.as_deref()); + let currency = args.currency.resolve(&ctx)?; + merge_context_currency(&mut request.context, currency.as_deref()); merge_pagination(&mut request, args.limit); let client = make_client(&ctx).await?; let response = decode::( diff --git a/rust/src/shopping/checkout/create.rs b/rust/src/shopping/checkout/create.rs index 3cf5a0a5..63b711af 100644 --- a/rust/src/shopping/checkout/create.rs +++ b/rust/src/shopping/checkout/create.rs @@ -7,9 +7,10 @@ use crate::output_schema::output_schema; use crate::shopping::AGENT_PURCHASE_CONFIRMATION_INSTRUCTIONS; use crate::shopping::SHOPPING_PURCHASE_SCOPES; use crate::shopping::common::{ - CheckoutInput, client_err, currency_code, make_client, no_saved_payment_method_action, + CheckoutInput, client_err, make_client, no_saved_payment_method_action, reject_multiple_payment_instruments, update_response, }; +use crate::shopping::currency::CurrencyArgs; use crate::shopping::human::{CHECKOUT_VIEW_ID, checkout_response, empty_acknowledgement_response}; output_schema!(CheckoutOutput { @@ -30,9 +31,8 @@ struct Args { #[arg(long, value_name = "PURCHASE_OPTION_ID[=QUANTITY]")] item: Vec, - /// Preferred ISO 4217 currency for checkout prices (for example, USD or GBP). - #[arg(long, value_name = "CODE", value_parser = currency_code)] - currency: Option, + #[command(flatten)] + currency: CurrencyArgs, /// Buyer's first name. #[arg(long, value_name = "NAME")] @@ -101,7 +101,7 @@ pub(super) fn command() -> RuntimeCommandSpec { |ctx, args: Args| async move { let input = CheckoutInput { items: args.item, - currency: args.currency, + currency: args.currency.resolve(&ctx)?, buyer_first_name: args.buyer_first_name, buyer_last_name: args.buyer_last_name, buyer_email: args.buyer_email, diff --git a/rust/src/shopping/currency.rs b/rust/src/shopping/currency.rs new file mode 100644 index 00000000..f7bee3f7 --- /dev/null +++ b/rust/src/shopping/currency.rs @@ -0,0 +1,150 @@ +//! `--currency`, shared by the shopping commands that price products. + +use cli_engine::CommandContext; + +use super::common::currency_code; +use crate::error::GddyError; +use crate::prefs::Prefs; + +/// Precedence: the flag, then `GDDY_CURRENCY`, then the saved `currency` +/// preference, then the catalog's own default. +#[derive(Debug, Clone, Default, clap::Args)] +pub(crate) struct CurrencyArgs { + /// Currency to show prices in, as an ISO 4217 code (for example, USD or GBP). Set a default + /// with `gddy config set currency `. + #[arg(long, env = "GDDY_CURRENCY", value_name = "CODE", value_parser = currency_code)] + currency: Option, +} + +impl CurrencyArgs { + /// The currency to request. The config file is only read when neither the + /// flag nor the environment variable decided, so an explicit choice works + /// even while the file is broken. + pub(crate) fn resolve(self, ctx: &CommandContext) -> Result, GddyError> { + match self.currency { + Some(explicit) => Ok(Some(explicit)), + None => Ok(Prefs::load(ctx)?.currency), + } + } +} + +#[cfg(test)] +mod tests { + use cli_engine::{ + Cli, CliConfig, CommandResult, CommandSpec, GroupSpec, Module, RuntimeCommandSpec, + RuntimeGroupSpec, + }; + use serde_json::json; + + use super::*; + use crate::environments::test_support::{ENV_LOCK, EnvGuard}; + + #[derive(Debug, Clone, clap::Args)] + struct ShowArgs { + #[command(flatten)] + currency: CurrencyArgs, + } + + fn cli() -> Cli { + let module = Module::new("Test", |_ctx| { + RuntimeGroupSpec::new(GroupSpec::new("t", "test")).with_command( + RuntimeCommandSpec::new_typed_with_context::( + CommandSpec::from_args::("show", "show").no_auth(true), + |ctx, args: ShowArgs| async move { + let currency = args.currency.resolve(&ctx)?; + Ok(CommandResult::new(json!({ "currency": currency }))) + }, + ), + ) + }); + Cli::new( + CliConfig::new("gddy", "test", "gddy") + .with_config_schema::() + .with_startup_args(Vec::<&str>::new()) + .with_module(module), + ) + } + + /// Runs `gddy t show ` with the given saved config and env var. + async fn run(config: Option<&str>, env: Option<&str>, flag: Option<&str>) -> (i32, String) { + let _lock = ENV_LOCK.lock().await; + let dir = tempfile::tempdir().expect("tempdir"); + let _xdg = EnvGuard::set("XDG_CONFIG_HOME", &dir.path().display().to_string()); + let _env = match env { + Some(value) => EnvGuard::set("GDDY_CURRENCY", value), + None => EnvGuard::unset("GDDY_CURRENCY"), + }; + if let Some(contents) = config { + let path = dir.path().join("gddy"); + std::fs::create_dir_all(&path).expect("mkdir"); + std::fs::write(path.join("config.toml"), contents).expect("write config"); + } + let mut args = vec!["gddy", "t", "show"]; + args.extend(flag.into_iter().flat_map(|f| f.split(' '))); + let out = cli().run(args).await; + (out.exit_code, out.rendered) + } + + async fn currency(config: Option<&str>, env: Option<&str>, flag: Option<&str>) -> String { + let (code, rendered) = run(config, env, flag).await; + assert_eq!( + code, 0, + "config={config:?} env={env:?} flag={flag:?}: {rendered}" + ); + let start = rendered.find("\"currency\":").expect("currency key"); + rendered[start..] + .lines() + .next() + .expect("line") + .trim_end_matches(',') + .to_owned() + } + + const SAVED: Option<&str> = Some("currency = \"GBP\"\n"); + + #[tokio::test] + async fn unset_by_default() { + assert_eq!(currency(None, None, None).await, "\"currency\": null"); + } + + #[tokio::test] + async fn saved_preference_applies_in_canonical_case() { + assert_eq!(currency(SAVED, None, None).await, "\"currency\": \"GBP\""); + assert_eq!( + currency(Some("currency = \"eur\"\n"), None, None).await, + "\"currency\": \"EUR\"" + ); + } + + #[tokio::test] + async fn env_beats_saved_preference() { + assert_eq!( + currency(SAVED, Some("jpy"), None).await, + "\"currency\": \"JPY\"" + ); + } + + #[tokio::test] + async fn flag_beats_env_and_saved_preference() { + assert_eq!( + currency(SAVED, Some("jpy"), Some("--currency usd")).await, + "\"currency\": \"USD\"" + ); + } + + #[tokio::test] + async fn explicit_choice_works_while_the_config_file_is_broken() { + let broken = Some("not = valid = toml"); + assert_eq!( + currency(broken, None, Some("--currency usd")).await, + "\"currency\": \"USD\"" + ); + } + + #[tokio::test] + async fn broken_saved_currency_is_an_error_not_a_silent_default() { + let (code, rendered) = run(Some("currency = \"banana\"\n"), None, None).await; + assert_ne!(code, 0, "{rendered}"); + assert!(rendered.contains("fix"), "{rendered}"); + } +} diff --git a/rust/src/shopping/guides/shopping.md b/rust/src/shopping/guides/shopping.md index 00c7513c..387f84b9 100644 --- a/rust/src/shopping/guides/shopping.md +++ b/rust/src/shopping/guides/shopping.md @@ -40,6 +40,8 @@ gddy shopping catalog search --category webHosting --category email `--limit` controls the number of products, not the number of purchase options. Use a product ID with `catalog get` or `catalog lookup`; use a purchase option ID when creating a checkout session. A requested currency is a preference—the currency returned with the price is authoritative. +If you always shop in one currency, save it once with `gddy config set currency GBP` (undo it with `gddy config unset currency`). `catalog search`, `catalog get`, `catalog lookup` and `checkout create` then use it unless you pass `--currency` or set the `GDDY_CURRENCY` environment variable; precedence is the flag, then the environment variable, then your saved setting. `checkout update` changes a session's currency only when you pass `--currency`, so a saved setting never re-prices an existing checkout session. + ```bash gddy shopping catalog get --currency GBP gddy shopping catalog lookup --currency GBP diff --git a/rust/src/shopping/mod.rs b/rust/src/shopping/mod.rs index bb1f01a0..f4a577bb 100644 --- a/rust/src/shopping/mod.rs +++ b/rust/src/shopping/mod.rs @@ -3,11 +3,14 @@ pub mod client; mod catalog; mod checkout; mod common; +mod currency; mod human; mod money; mod order; mod product_actions; +pub(crate) use common::currency_code; + use cli_engine::{GroupSpec, Module, RuntimeGroupSpec}; use crate::scopes::{SHOPPING_CATALOG_READ, SHOPPING_CHECKOUT_EXECUTE, SHOPPING_ORDER_READ};