diff --git a/hive-forge/src/verbs/attach.rs b/hive-forge/src/verbs/attach.rs index 8819a879..ec05f23b 100644 --- a/hive-forge/src/verbs/attach.rs +++ b/hive-forge/src/verbs/attach.rs @@ -51,7 +51,7 @@ pub fn run_issue(client: &Client, args: IssueArgs) -> Result<()> { .api() .issue_create_issue_attachment(owner, name, index(args.number)?, &bytes, query) .send()?; - print_url(&resp); + println!("{}", require_url(&resp)?); Ok(()) } @@ -78,7 +78,7 @@ pub fn run_comment(client: &Client, args: CommentArgs) -> Result<()> { .api() .issue_create_issue_comment_attachment(owner, name, index(args.id)?, &bytes, query) .send()?; - print_url(&resp); + println!("{}", require_url(&resp)?); Ok(()) } @@ -93,8 +93,68 @@ fn file_name(path: &Path) -> Option { path.file_name().map(|n| n.to_string_lossy().into_owned()) } -fn print_url(v: &Attachment) { - if let Some(url) = &v.browser_download_url { - println!("{url}"); +/// The uploaded attachment's browser download URL, hard-erroring if the +/// forge answered 2xx with none. A caller piping the printed URL (e.g. +/// `url=$(hive-forge attach-issue …)`) used to get an empty string and no +/// signal when this was `None` — the upload had happened, but nothing said +/// so, and nothing said it hadn't. +fn require_url(v: &Attachment) -> Result { + v.browser_download_url.clone().ok_or_else(|| { + anyhow::anyhow!( + "hive-forge attach: upload succeeded but the forge returned no download URL \ + (attachment {})", + attachment_ident(v) + ) + }) +} + +/// Best-effort identifier for an attachment error message — id if present, +/// else name, else a placeholder so the message never comes out empty. +fn attachment_ident(v: &Attachment) -> String { + match (v.id, v.name.as_deref()) { + (Some(id), Some(n)) => format!("id={id} name={n}"), + (Some(id), None) => format!("id={id}"), + (None, Some(n)) => format!("name={n}"), + (None, None) => "".to_owned(), + } +} + +#[cfg(test)] +mod tests { + use super::require_url; + use forgejo_api::structs::Attachment; + + fn attachment(url: Option<&str>) -> Attachment { + Attachment { + browser_download_url: url.map(|u| u.parse().unwrap()), + created_at: None, + download_count: None, + id: Some(7), + name: Some("out.log".to_owned()), + size: None, + r#type: None, + uuid: None, + } + } + + #[test] + fn control_present_url_is_returned() { + let a = attachment(Some("https://forge.example/a/1")); + assert_eq!( + require_url(&a).unwrap().as_str(), + "https://forge.example/a/1" + ); + } + + #[test] + fn missing_url_errors_naming_the_attachment() { + // A 2xx response with no browser_download_url used to print nothing + // and exit 0 — a caller doing `url=$(hive-forge attach-issue …)` + // got an empty string with no signal anything went wrong. + let a = attachment(None); + let err = require_url(&a).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("id=7"), "missing attachment id: {msg}"); + assert!(msg.contains("out.log"), "missing attachment name: {msg}"); } }