Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes

## Unreleased

- A list that returns every item in one response, such as `wallets list`, accepts `--all` and ignores it, because there are no more pages. Before, it rejected `--all`. `--all` still pages through lists that take `--limit`.
- A command that acts on one wallet, such as `wallets get`, also takes the wallet as `--wallet ID`. If the argument and `--wallet` name different wallets, the command stops. Without either, the error names both ways to pass it. A profile's or `VOLTAGE_WALLET_ID`'s wallet is never used, so a delete always names its wallet.
- A request that gets no response says why: a timeout (for the auth service, its fixed 30-second limit, which `--timeout` does not change), a connection not made within the 10-second connect limit (which `--timeout` does not change), a failed TLS connection, a connection that could not be made, or one that closed early. A hint names the next step. A write that timed out or closed early may have been applied, so its hint says to look up the resource ID before running the command again, never to retry. This covers API, auth, price, and checkout stream requests, which before said only that the request failed.
- `--output-file` on a path that already exists says so and suggests a new path, instead of showing an OS error; any other refusal keeps its cause and gets the same hint. Both say that no change was sent.
Expand Down
38 changes: 35 additions & 3 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,24 @@ fn add_children(mut parent: ClapCommand, prefix: &[String]) -> ClapCommand {
}

/// The command for one operation: its positional target, documented filters, and options.
/// `--all` pages through a list that takes `--limit`. A list without paging accepts `--all`
/// too and ignores it, because its one response already holds every item.
fn list_args(cmd: ClapCommand, operation: &Operation) -> ClapCommand {
if operation.has_parameter("limit") {
PaginationFlags::augment_args(cmd)
} else if operation.lists_everything_at_once() {
cmd.arg(
Arg::new("all_at_once")
.long("all")
.action(ArgAction::SetTrue)
.help("No effect: this list returns every item in one response")
.help_heading("Query"),
)
} else {
cmd
}
}

/// The resource ID argument and `--id`, one of which is required. A wallet command can
/// name its wallet with `--wallet` instead, so `target_id` checks that one was given.
fn target_args(cmd: ClapCommand, target: ResourceTarget, wallet: bool) -> ClapCommand {
Expand Down Expand Up @@ -989,9 +1007,7 @@ fn endpoint(mut cmd: ClapCommand, operation: &Operation, about: String) -> ClapC
if operation.method != Method::Get {
cmd = ExecuteFlags::augment_args(cmd);
}
if operation.has_parameter("limit") {
cmd = PaginationFlags::augment_args(cmd);
}
cmd = list_args(cmd, operation);
if operation.id.lists_all_organizations() {
cmd = AllOrgsFlags::augment_args(cmd);
}
Expand Down Expand Up @@ -1421,6 +1437,22 @@ mod tests {
);
}

#[test]
fn all_is_accepted_by_every_list_and_pages_only_a_paginated_one() {
// `wallets list` returns every wallet at once, so `--all` changes nothing.
assert!(!api(&["wallets", "list", "--all"]).all);
assert!(!api(&["webhooks", "list", "--all"]).all);
assert!(api(&["payments", "list", "--all"]).all);
// A command that is not a list still rejects it.
let get =
Invocation::try_parse_from(["voltage", "wallets", "get", WALLET, "--all"]).unwrap_err();
assert!(
get.message.contains("unexpected argument '--all'"),
"{}",
get.message
);
}

#[test]
fn wallet_flag_names_the_wallet_a_wallet_command_acts_on() {
const OTHER: &str = "44444444-4444-4444-8444-444444444444";
Expand Down
26 changes: 26 additions & 0 deletions src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,14 @@ impl Operation {
})
}

/// A `list` command whose one response holds every item: it has no `--limit` to page.
pub fn lists_everything_at_once(&self) -> bool {
self.method == Method::Get
&& self.target.is_none()
&& self.command.last().is_some_and(|word| word == "list")
&& !self.has_parameter("limit")
}

pub fn has_parameter(&self, name: &str) -> bool {
self.parameters
.iter()
Expand Down Expand Up @@ -512,6 +520,24 @@ mod tests {
use super::*;
use std::collections::BTreeSet;

#[test]
fn only_lists_without_paging_return_everything_at_once() {
for id in [
OperationId::GetAllOrganizationsWalletsAsUser,
OperationId::GetWebhooksAsUser,
] {
assert!(operation(id).lists_everything_at_once(), "{id:?}");
}
// A paginated list, a single read, and a change are not.
for id in [
OperationId::GetPayments,
OperationId::GetWallet,
OperationId::CreateWallet,
] {
assert!(!operation(id).lists_everything_at_once(), "{id:?}");
}
}

#[test]
fn registry_matches_the_contract() {
let ids: BTreeSet<_> = OPERATIONS.iter().map(|operation| operation.id).collect();
Expand Down
Loading