diff --git a/Cargo.lock b/Cargo.lock index 2abc03f..4162442 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9,18 +9,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" [[package]] -name = "alloc-no-stdlib" -version = "2.0.4" +name = "aho-corasick" +version = "1.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cc7bb162ec39d46ab1ca8c77bf72e890535becd1751bb45f64c597edb4c8c6b3" - -[[package]] -name = "alloc-stdlib" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "94fb8275041c72129eb51b7d0322c29b8387a0386127718b096429201a5d6ece" +checksum = "ddd31a130427c27518df266943a5308ed92d4b226cc639f5a8f1002816174301" dependencies = [ - "alloc-no-stdlib", + "memchr", ] [[package]] @@ -113,6 +107,16 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "23b62fc65de8e4e7f52534fb52b0f3ed04746ae267519eef2a83941e8085068b" +[[package]] +name = "assert-json-diff" +version = "2.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47e4f2b81832e72834d7518d8487a0396a28cc408186a2e8854c0f98011faf12" +dependencies = [ + "serde", + "serde_json", +] + [[package]] name = "async-compression" version = "0.4.42" @@ -149,27 +153,6 @@ version = "2.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4512299f36f043ab09a583e57bceb5a5aab7a73db1805848e8fef3c9e8c78b3" -[[package]] -name = "brotli" -version = "8.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4bd8b9603c7aa97359dbd97ecf258968c95f3adddd6db2f7e7a5bef101c84560" -dependencies = [ - "alloc-no-stdlib", - "alloc-stdlib", - "brotli-decompressor", -] - -[[package]] -name = "brotli-decompressor" -version = "5.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "874bb8112abecc98cbd6d81ea4fa7e94fb9449648c93cc89aa40c81c24d7de03" -dependencies = [ - "alloc-no-stdlib", - "alloc-stdlib", -] - [[package]] name = "bumpalo" version = "3.20.2" @@ -300,7 +283,6 @@ version = "0.4.38" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ce2548391e9c1929c21bf6aa2680af86fe4c1b33e6cea9ac1cfeec0bd11218cf" dependencies = [ - "brotli", "compression-core", "flate2", "memchr", @@ -360,6 +342,24 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "deadpool" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0be2b1d1d6ec8d846f05e137292d0b89133caf95ef33695424c09568bdd39b1b" +dependencies = [ + "deadpool-runtime", + "lazy_static", + "num_cpus", + "tokio", +] + +[[package]] +name = "deadpool-runtime" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "092966b41edc516079bdf31ec78a2e0588d1d0c08f78b91d8307215928642b2b" + [[package]] name = "dialoguer" version = "0.11.0" @@ -443,9 +443,6 @@ dependencies = [ "clap_mangen", "dialoguer", "directories", - "futures-util", - "indicatif", - "is-terminal", "keyring", "libc", "owo-colors", @@ -454,11 +451,11 @@ dependencies = [ "serde_json", "supports-color 3.0.2", "tabled", - "textwrap", "thiserror 2.0.18", "tokio", "toml", "url", + "wiremock", ] [[package]] @@ -486,6 +483,21 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "futures" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8b147ee9d1f6d097cef9ce628cd2ee62288d963e16fb287bd9286455b241382d" +dependencies = [ + "futures-channel", + "futures-core", + "futures-executor", + "futures-io", + "futures-sink", + "futures-task", + "futures-util", +] + [[package]] name = "futures-channel" version = "0.3.32" @@ -493,6 +505,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "07bbe89c50d7a535e539b8c17bc0b49bdb77747034daa8087407d655f3f7cc1d" dependencies = [ "futures-core", + "futures-sink", ] [[package]] @@ -501,6 +514,17 @@ version = "0.3.32" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7e3450815272ef58cec6d564423f6e755e25379b217b0bc688e295ba24df6b1d" +[[package]] +name = "futures-executor" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "baf29c38818342a3b26b5b923639e7b1f4a61fc5e76102d4b1981c6dc7a7579d" +dependencies = [ + "futures-core", + "futures-task", + "futures-util", +] + [[package]] name = "futures-io" version = "0.3.32" @@ -536,6 +560,7 @@ version = "0.3.32" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "389ca41296e6190b48053de0321d02a77f32f8a5d2461dd38762c0593805c6d6" dependencies = [ + "futures-channel", "futures-core", "futures-io", "futures-macro", @@ -573,6 +598,25 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "h2" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "171fefbc92fe4a4de27e0698d6a5b392d6a0e333506bc49133760b3bcf948733" +dependencies = [ + "atomic-waker", + "bytes", + "fnv", + "futures-core", + "futures-sink", + "http", + "indexmap", + "slab", + "tokio", + "tokio-util", + "tracing", +] + [[package]] name = "hashbrown" version = "0.17.1" @@ -636,6 +680,12 @@ version = "1.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6dbf3de79e51f3d586ab4cb9d5c3e2c14aa28ed23d180cf89b4df0454a69cc87" +[[package]] +name = "httpdate" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9" + [[package]] name = "hyper" version = "1.9.0" @@ -646,9 +696,11 @@ dependencies = [ "bytes", "futures-channel", "futures-core", + "h2", "http", "http-body", "httparse", + "httpdate", "itoa", "pin-project-lite", "smallvec", @@ -832,19 +884,6 @@ dependencies = [ "hashbrown", ] -[[package]] -name = "indicatif" -version = "0.17.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "183b3088984b400f4cfac3620d5e076c84da5364016b4f49473de574b2586235" -dependencies = [ - "console", - "number_prefix", - "portable-atomic", - "unicode-width 0.2.2", - "web-time", -] - [[package]] name = "ipnet" version = "2.12.0" @@ -907,6 +946,12 @@ dependencies = [ "zeroize", ] +[[package]] +name = "lazy_static" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" + [[package]] name = "libc" version = "0.2.186" @@ -1025,10 +1070,14 @@ dependencies = [ ] [[package]] -name = "number_prefix" -version = "0.4.0" +name = "num_cpus" +version = "1.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "830b246a0e5f20af87141b25c173cd1b609bd7779a4617d6ec582abaf90870f3" +checksum = "91df4bbde75afed763b708b7eee1e8e7651e02d97f6d5dd763e89367e957b23b" +dependencies = [ + "hermit-abi", + "libc", +] [[package]] name = "once_cell" @@ -1083,12 +1132,6 @@ version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" -[[package]] -name = "portable-atomic" -version = "1.13.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c33a9471896f1c69cecef8d20cbe2f7accd12527ce60845ff44c153bb2a21b49" - [[package]] name = "potential_utf" version = "0.1.5" @@ -1250,6 +1293,35 @@ dependencies = [ "thiserror 1.0.69", ] +[[package]] +name = "regex" +version = "1.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e10754a14b9137dd7b1e3e5b0493cc9171fdd105e0ab477f51b72e7f3ac0e276" +dependencies = [ + "aho-corasick", + "memchr", + "regex-automata", + "regex-syntax", +] + +[[package]] +name = "regex-automata" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e1dd4122fc1595e8162618945476892eefca7b88c52820e74af6262213cae8f" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax", +] + +[[package]] +name = "regex-syntax" +version = "0.8.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc897dd8d9e8bd1ed8cdad82b5966c3e0ecae09fb1907d58efaa013543185d0a" + [[package]] name = "reqwest" version = "0.12.28" @@ -1280,14 +1352,12 @@ dependencies = [ "sync_wrapper", "tokio", "tokio-rustls", - "tokio-util", "tower", "tower-http", "tower-service", "url", "wasm-bindgen", "wasm-bindgen-futures", - "wasm-streams", "web-sys", "webpki-roots", ] @@ -1518,12 +1588,6 @@ version = "1.15.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "67b1b7a3b5fe4f1376887184045fcf45c69e92af734b7aaddc05fb777b6fbd03" -[[package]] -name = "smawk" -version = "0.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b7c388c1b5e93756d0c740965c41e8822f866621d41acbdf6336a6a168f8840c" - [[package]] name = "socket2" version = "0.6.3" @@ -1648,17 +1712,6 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "textwrap" -version = "0.16.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c13547615a44dc9c452a8a534638acdf07120d4b6847c8178705da06306a3057" -dependencies = [ - "smawk", - "unicode-linebreak", - "unicode-width 0.2.2", -] - [[package]] name = "thiserror" version = "1.0.69" @@ -1902,12 +1955,6 @@ version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" -[[package]] -name = "unicode-linebreak" -version = "0.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b09c83c3c29d37506a3e260c08c03743a6bb66a9cd432c6934ab501a190571f" - [[package]] name = "unicode-width" version = "0.1.11" @@ -2056,19 +2103,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "wasm-streams" -version = "0.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "15053d8d85c7eccdbefef60f06769760a563c7f0a9d6902a13d35c7800b0ad65" -dependencies = [ - "futures-util", - "js-sys", - "wasm-bindgen", - "wasm-bindgen-futures", - "web-sys", -] - [[package]] name = "web-sys" version = "0.3.98" @@ -2397,6 +2431,29 @@ dependencies = [ "memchr", ] +[[package]] +name = "wiremock" +version = "0.6.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08db1edfb05d9b3c1542e521aea074442088292f00b5f28e435c714a98f85031" +dependencies = [ + "assert-json-diff", + "base64", + "deadpool", + "futures", + "http", + "http-body-util", + "hyper", + "hyper-util", + "log", + "once_cell", + "regex", + "serde", + "serde_json", + "tokio", + "url", +] + [[package]] name = "wit-bindgen" version = "0.57.1" diff --git a/Cargo.toml b/Cargo.toml index eb51f38..6cac6e8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ clap = { version = "4.5", features = ["derive", "env", "wrap_help"] } clap_complete = "4.5" clap_mangen = "0.2" tokio = { version = "1", features = ["rt-multi-thread", "macros", "fs", "process", "io-util", "io-std", "signal"] } -reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls", "stream", "gzip", "brotli", "multipart"] } +reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls", "gzip", "multipart"] } serde = { version = "1", features = ["derive"] } serde_json = "1" toml = "0.8" @@ -31,19 +31,19 @@ chrono = { version = "0.4", default-features = false, features = ["serde", "cloc tabled = { version = "0.16", features = ["ansi"] } owo-colors = { version = "4", features = ["supports-colors"] } supports-color = "3" -indicatif = "0.17" dialoguer = { version = "0.11", default-features = false, features = ["password"] } -is-terminal = "0.4" -textwrap = "0.16" -futures-util = "0.3" libc = "0.2" +[dev-dependencies] +wiremock = "0.6" +tokio = { version = "1", features = ["rt-multi-thread", "macros", "test-util"] } + [profile.release] -lto = "thin" +lto = "fat" codegen-units = 1 strip = "symbols" opt-level = 3 +panic = "abort" [profile.dist] inherits = "release" -lto = "fat" diff --git a/src/cli/mod.rs b/src/cli/mod.rs index c801180..5048cc0 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -21,7 +21,7 @@ pub mod status; pub mod web; pub mod workflow; -use anyhow::Result; +use anyhow::{Context, Result}; use clap::{Parser, Subcommand}; /// `fj` — command-line tool for Forgejo. @@ -120,14 +120,9 @@ pub struct CompletionArgs { pub async fn run(cli: Cli) -> Result<()> { crate::client::set_debug(cli.debug); - if cli.no_pager { - // SAFETY: setting an env var is safe; child processes inherit it. - unsafe { std::env::set_var("FJ_NO_PAGER", "1") }; - } - // Page commands whose output can run long: any list, view, diff, or - // commits. Each call site re-enters this helper if it wants pager support. - // We start it conditionally here too so most `--help` invocations stay - // unpaged. + // Page commands whose output can run long. Pass `cli.no_pager` through + // explicitly so we don't have to mutate the process environment to opt + // out, which would be a process-wide side effect and an unsafe set_var. let _pager = match &cli.command { Command::Repo(_) | Command::Issue(_) @@ -137,33 +132,49 @@ pub async fn run(cli: Cli) -> Result<()> { | Command::Status(_) | Command::Label(_) | Command::Run(_) - | Command::Api(_) => crate::output::pager::maybe_start(), + | Command::Api(_) => crate::output::pager::maybe_start(cli.no_pager), _ => None, }; - match cli.command { + // Run the command, racing it against Ctrl+C. On SIGINT we return an error + // and let `_pager` drop naturally — its Drop impl restores stdout and + // waits on the pager child, leaving the terminal in a clean state. + let cli_host = cli.host.clone(); + let cmd_future = dispatch(cli.command, cli_host.as_deref()); + tokio::select! { + biased; + sig = tokio::signal::ctrl_c() => { + sig.context("installing SIGINT handler")?; + Err(anyhow::anyhow!("interrupted")) + } + res = cmd_future => res, + } +} + +async fn dispatch(command: Command, host: Option<&str>) -> Result<()> { + match command { Command::Auth(cmd) => auth::run(cmd).await, - Command::Repo(cmd) => repo::run(cmd, cli.host.as_deref()).await, - Command::Issue(cmd) => issue::run(cmd, cli.host.as_deref()).await, - Command::Pr(cmd) => pr::run(cmd, cli.host.as_deref()).await, - Command::Release(cmd) => release::run(cmd, cli.host.as_deref()).await, - Command::Label(cmd) => label::run(cmd, cli.host.as_deref()).await, - Command::Run(cmd) => workflow::run(cmd, cli.host.as_deref()).await, - Command::Secret(cmd) => workflow::run_secret(cmd, cli.host.as_deref()).await, - Command::Variable(cmd) => workflow::run_variable(cmd, cli.host.as_deref()).await, - Command::Search(cmd) => search::run(cmd, cli.host.as_deref()).await, - Command::Browse(args) => browse::run(args, cli.host.as_deref()).await, - Command::Status(args) => status::run(args, cli.host.as_deref()).await, - Command::Org(cmd) => org::run(cmd, cli.host.as_deref()).await, - Command::SshKey(cmd) => key::run_ssh(cmd, cli.host.as_deref()).await, - Command::GpgKey(cmd) => key::run_gpg(cmd, cli.host.as_deref()).await, + Command::Repo(cmd) => repo::run(cmd, host).await, + Command::Issue(cmd) => issue::run(cmd, host).await, + Command::Pr(cmd) => pr::run(cmd, host).await, + Command::Release(cmd) => release::run(cmd, host).await, + Command::Label(cmd) => label::run(cmd, host).await, + Command::Run(cmd) => workflow::run(cmd, host).await, + Command::Secret(cmd) => workflow::run_secret(cmd, host).await, + Command::Variable(cmd) => workflow::run_variable(cmd, host).await, + Command::Search(cmd) => search::run(cmd, host).await, + Command::Browse(args) => browse::run(args, host).await, + Command::Status(args) => status::run(args, host).await, + Command::Org(cmd) => org::run(cmd, host).await, + Command::SshKey(cmd) => key::run_ssh(cmd, host).await, + Command::GpgKey(cmd) => key::run_gpg(cmd, host).await, Command::Alias(cmd) => alias::run(cmd).await, Command::Config(cmd) => config::run(cmd).await, - Command::Protect(cmd) => protect::run(cmd, cli.host.as_deref()).await, - Command::Hook(cmd) => hook::run(cmd, cli.host.as_deref()).await, + Command::Protect(cmd) => protect::run(cmd, host).await, + Command::Hook(cmd) => hook::run(cmd, host).await, Command::Extension(cmd) => extension::run(cmd).await, - Command::Gist(cmd) => gist::run(cmd, cli.host.as_deref()).await, - Command::Api(args) => api::run(args, cli.host.as_deref()).await, + Command::Gist(cmd) => gist::run(cmd, host).await, + Command::Api(args) => api::run(args, host).await, Command::Completion(args) => { use clap::CommandFactory; let mut cmd = Cli::command(); diff --git a/src/client/integration_tests.rs b/src/client/integration_tests.rs new file mode 100644 index 0000000..94dab86 --- /dev/null +++ b/src/client/integration_tests.rs @@ -0,0 +1,208 @@ +//! Integration tests for the HTTP client against a local wiremock server. +//! Lives inline (not under `tests/`) because our crate is a binary, not a +//! library — `tests/` can't `use crate::client::...`. + +#![cfg(test)] + +use reqwest::Method; +use serde_json::json; +use url::Url; +use wiremock::matchers::{header, method, path, query_param}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +use super::Client; + +async fn client_for(server: &MockServer) -> Client { + let base = Url::parse(&format!("{}/api/v1/", server.uri())).expect("base url"); + Client::for_base_url(base, "test-token".into()).expect("client") +} + +#[tokio::test] +async fn auth_header_sent() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/api/v1/user")) + .and(header("authorization", "token test-token")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "id": 1, "login": "alice" + }))) + .expect(1) + .mount(&server) + .await; + let c = client_for(&server).await; + let res: serde_json::Value = c + .json(Method::GET, "/api/v1/user", &[], None::<&()>) + .await + .expect("ok"); + assert_eq!(res["login"], "alice"); +} + +#[tokio::test] +async fn retries_on_5xx_and_succeeds() { + let server = MockServer::start().await; + // First two attempts: 503. Third: 200. + Mock::given(method("GET")) + .and(path("/api/v1/flaky")) + .respond_with(ResponseTemplate::new(503)) + .up_to_n_times(2) + .expect(2) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/api/v1/flaky")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({"ok": true}))) + .expect(1) + .mount(&server) + .await; + let c = client_for(&server).await; + let res: serde_json::Value = c + .json(Method::GET, "/api/v1/flaky", &[], None::<&()>) + .await + .expect("eventually ok"); + assert_eq!(res["ok"], true); +} + +#[tokio::test] +async fn does_not_retry_post_on_5xx() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/api/v1/once")) + .respond_with(ResponseTemplate::new(500)) + .expect(1) + .mount(&server) + .await; + let c = client_for(&server).await; + let err = c + .json::(Method::POST, "/api/v1/once", &[], Some(&json!({}))) + .await; + assert!(err.is_err()); +} + +#[tokio::test] +async fn maps_401_to_friendly_error() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/api/v1/user")) + .respond_with(ResponseTemplate::new(401).set_body_json(json!({"message": "Bad token"}))) + .mount(&server) + .await; + let c = client_for(&server).await; + let err = c + .json::(Method::GET, "/api/v1/user", &[], None::<&()>) + .await + .expect_err("should fail"); + let text = format!("{err:#}"); + assert!(text.contains("401"), "got: {text}"); + assert!( + text.to_lowercase().contains("authentication") || text.to_lowercase().contains("token"), + "got: {text}" + ); +} + +#[tokio::test] +async fn custom_headers_forwarded() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/api/v1/echo")) + .and(header("x-trace-id", "abc-123")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({"seen": true}))) + .expect(1) + .mount(&server) + .await; + let c = client_for(&server).await; + let mut h = reqwest::header::HeaderMap::new(); + h.insert("x-trace-id", "abc-123".parse().unwrap()); + let res = c + .request_with_headers(Method::GET, "/api/v1/echo", &[], None, &h) + .await + .expect("ok"); + assert!(res.status().is_success()); +} + +#[tokio::test] +async fn get_page_handles_null_body() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/api/v1/things")) + .respond_with(ResponseTemplate::new(200).set_body_string("null")) + .expect(1) + .mount(&server) + .await; + let c = client_for(&server).await; + let page: super::Page = c + .get_page("/api/v1/things", &[]) + .await + .expect("null tolerated"); + assert!(page.items.is_empty()); +} + +#[tokio::test] +async fn get_all_follows_link_rel_next() { + let server = MockServer::start().await; + // First page: 50 items, Link: <…?page=2>; rel="next" + let page1: Vec = (1..=50).collect(); + let page2: Vec = (51..=80).collect(); + let next_url = format!("{}/api/v1/items?page=2&limit=50", server.uri()); + // Page 2 mock first (more specific). wiremock matches in LIFO order, so + // we mount the more specific mock LAST to make sure it wins. + Mock::given(method("GET")) + .and(path("/api/v1/items")) + .and(query_param("page", "2")) + .respond_with(ResponseTemplate::new(200).set_body_json(page2)) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/api/v1/items")) + .and(query_param("limit", "50")) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(page1) + .insert_header("Link", format!(r#"<{next_url}>; rel="next""#).as_str()), + ) + .expect(1) + .mount(&server) + .await; + + let c = client_for(&server).await; + let items: Vec = c + .get_all("/api/v1/items", &[], 100) + .await + .expect("paginated ok"); + assert_eq!(items.len(), 80); + assert_eq!(items.first(), Some(&1)); + assert_eq!(items.last(), Some(&80)); +} + +#[tokio::test] +async fn get_all_respects_total_limit() { + let server = MockServer::start().await; + let big: Vec = (1..=50).collect(); + let next_url = format!("{}/api/v1/items?page=2&limit=50", server.uri()); + Mock::given(method("GET")) + .and(path("/api/v1/items")) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(big) + .insert_header("Link", format!(r#"<{next_url}>; rel="next""#).as_str()), + ) + .expect(1) // exactly one call: the limit caps the walk early + .mount(&server) + .await; + let c = client_for(&server).await; + let items: Vec = c.get_all("/api/v1/items", &[], 30).await.expect("ok"); + assert_eq!(items.len(), 30); +} + +#[tokio::test] +async fn header_value_rejects_bad_token() { + let server = MockServer::start().await; + let base = Url::parse(&format!("{}/api/v1/", server.uri())).unwrap(); + let bad = Client::for_base_url(base, "tok\nen".into()).expect("constructed"); + let err = bad + .json::(Method::GET, "/api/v1/user", &[], None::<&()>) + .await + .expect_err("bad token should not panic"); + let text = format!("{err:#}"); + assert!(text.contains("clean token"), "got: {text}"); +} diff --git a/src/client/mod.rs b/src/client/mod.rs index 461d303..f6d89cf 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -1,4 +1,6 @@ pub mod error; +#[cfg(test)] +mod integration_tests; pub mod pagination; use std::sync::atomic::{AtomicBool, Ordering}; @@ -85,6 +87,25 @@ impl Client { Self::new(resolve(host_flag)?) } + /// Construct a client pointing at an arbitrary base URL. Used for tests + /// against a local mock server. `base_url` must end with `/`. + #[allow(dead_code)] + pub fn for_base_url(base_url: Url, token: String) -> Result { + let host = base_url.host_str().map(str::to_string).unwrap_or_default(); + let http = reqwest::Client::builder() + .user_agent(default_user_agent()) + .connect_timeout(Duration::from_secs(15)) + .timeout(Duration::from_secs(60)) + .build() + .context("building HTTP client")?; + Ok(Self { + http, + host, + base: base_url, + token, + }) + } + #[allow(dead_code)] pub fn host(&self) -> &str { &self.host @@ -106,19 +127,22 @@ impl Client { &self.token } - fn auth_headers(&self) -> HeaderMap { + fn auth_headers(&self) -> Result { let mut headers = HeaderMap::new(); - headers.insert( - AUTHORIZATION, - HeaderValue::from_str(&format!("token {}", self.token)) - .expect("token contains invalid header chars"), - ); + let value = HeaderValue::from_str(&format!("token {}", self.token)).map_err(|_| { + anyhow!( + "stored token for host '{}' contains characters that can't appear in an HTTP header. Re-run `fj auth login --host {}` with a clean token.", + self.host, + self.host + ) + })?; + headers.insert(AUTHORIZATION, value); headers.insert(ACCEPT, HeaderValue::from_static("application/json")); headers.insert( USER_AGENT, HeaderValue::from_static(default_user_agent_static()), ); - headers + Ok(headers) } /// Build an absolute URL for an API path. Accepts any of: a full URL @@ -149,6 +173,10 @@ impl Client { } /// Like `request` but merges `extra` headers in (they override defaults). + /// + /// The request is constructed once and cloned per retry attempt via + /// `reqwest::Request::try_clone`, which avoids re-allocating headers and + /// re-parsing the URL on each attempt. pub async fn request_with_headers( &self, method: Method, @@ -158,42 +186,42 @@ impl Client { extra: &HeaderMap, ) -> Result { let url = self.url(path)?; - let mut headers = self.auth_headers(); + let mut headers = self.auth_headers()?; for (k, v) in extra.iter() { headers.insert(k.clone(), v.clone()); } + if debug_enabled() { - let q = if query.is_empty() { - String::new() - } else { - let pairs: Vec = query.iter().map(|(k, v)| format!("{k}={v}")).collect(); - format!("?{}", pairs.join("&")) - }; - eprintln!("→ {method} {url}{q}"); - if let Some(b) = body { - if let Ok(text) = serde_json::to_string(b) { - let preview = if text.len() > 200 { - format!("{}…", &text[..200]) - } else { - text - }; - eprintln!(" body: {preview}"); - } - } + log_request(&method, &url, query, body); } let retries = if is_idempotent(&method) { 3 } else { 1 }; + + // Build the request once. `Request::try_clone` (used per retry below) + // succeeds for any request whose body is `None`, `Bytes`, or `Text`. + // We only ever set a JSON body, which lives in memory as bytes, so the + // clone is always successful. + let mut builder = self + .http + .request(method.clone(), url.clone()) + .headers(headers) + .query(query); + if let Some(body) = body { + builder = builder.json(body); + } + let prepared = builder.build().context("building request")?; + let mut last_err: Option = None; for attempt in 0..retries { - let mut req = self - .http - .request(method.clone(), url.clone()) - .headers(headers.clone()) - .query(query); - if let Some(body) = body { - req = req.json(body); - } - match req.send().await { + let req = match prepared.try_clone() { + Some(c) => c, + None => { + return Err(anyhow!( + "internal: request body could not be cloned for retry" + )); + } + }; + match self.http.execute(req).await { Ok(res) => { let status = res.status(); if status.is_server_error() && attempt + 1 < retries { @@ -355,6 +383,34 @@ async fn backoff(attempt: u32) { tokio::time::sleep(std::time::Duration::from_millis(delay)).await; } +/// Emit a `--debug` log line for an outgoing request. Pulled out of the hot +/// path so the formatting code is gated behind the cold `debug_enabled` check. +#[cold] +fn log_request( + method: &Method, + url: &Url, + query: &[(String, String)], + body: Option<&serde_json::Value>, +) { + let q = if query.is_empty() { + String::new() + } else { + let pairs: Vec = query.iter().map(|(k, v)| format!("{k}={v}")).collect(); + format!("?{}", pairs.join("&")) + }; + eprintln!("→ {method} {url}{q}"); + if let Some(b) = body { + if let Ok(text) = serde_json::to_string(b) { + let preview = if text.len() > 200 { + format!("{}…", &text[..200]) + } else { + text + }; + eprintln!(" body: {preview}"); + } + } +} + fn build_base_url(hostname: &str, host: &Host) -> Result { let scheme = "https"; let mut url = Url::parse(&format!("{scheme}://{hostname}")) diff --git a/src/output/pager.rs b/src/output/pager.rs index 37ac71d..e7c1163 100644 --- a/src/output/pager.rs +++ b/src/output/pager.rs @@ -44,9 +44,9 @@ impl Drop for PagerGuard { /// failed. /// /// The returned guard must outlive all `println!`/`print!` calls produced by -/// the current command. -pub fn maybe_start() -> Option { - if std::env::var_os("FJ_NO_PAGER").is_some() { +/// the current command. Pass `force_disabled=true` to honor `--no-pager`. +pub fn maybe_start(force_disabled: bool) -> Option { + if force_disabled || std::env::var_os("FJ_NO_PAGER").is_some() { return None; } if !std::io::stdout().is_terminal() {