diff --git a/CHANGELOG.md b/CHANGELOG.md index 7428211..a791206 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -80,6 +80,15 @@ that may never merge. They are not releases and are not listed here. that passes them now fails with an unknown-argument error; drop the flags. `mapbox search forward` keeps them. (#76) +- `mapbox places ...`, full place detail — hours, phone, + website, photos, address, coordinates, activity data — for one or more + `mapbox_id`s a Search Box API result already returned, up to 100 in one + call. Always uses the batch endpoint, even for a single id, so there is + one command rather than a choice between one id and a hand-typed JSON + array. Hand-authored into `custom-openapi/` since no upstream spec + exists yet. Places is Public Preview, with a 1000-records-per-account + monthly quota. + - `mapbox matrix`, travel time and/or distance between every pair in a set of up to 25 coordinates in one call, for driving (with or without live traffic), walking, or cycling. No subcommand: like `mapbox directions` diff --git a/README.md b/README.md index 29734ae..e013cab 100644 --- a/README.md +++ b/README.md @@ -251,11 +251,11 @@ mapbox styles mapbox tilesets ``` -`mapbox directions`, `mapbox isochrone`, `mapbox map-match`, and -`mapbox matrix` are the exceptions: each API has a single operation, so -there's a bare command with no subcommand at all, the same shape -`mapbox usage` already has, see [docs/commands.md](./docs/commands.md) -for their own parameters. +`mapbox directions`, `mapbox isochrone`, `mapbox map-match`, +`mapbox matrix`, and `mapbox places` are the exceptions: each API has a +single exposed operation, so there's a bare command with no subcommand at +all, the same shape `mapbox usage` already has, see +[docs/commands.md](./docs/commands.md) for their own parameters. A command group is not the same thing as a spec file: which one an operation belongs to is decided per operation. So `sprites` and `tilesets` are each diff --git a/custom-openapi/places/openapi/places.yaml b/custom-openapi/places/openapi/places.yaml new file mode 100644 index 0000000..52f00e3 --- /dev/null +++ b/custom-openapi/places/openapi/places.yaml @@ -0,0 +1,100 @@ +openapi: "3.0.0" +# `parse_spec` turns `info.description` below into this service's clap +# `long_about`, so it also reaches `mapbox places --help`, `--schema` and +# `generate-skills` output. Keep it to API prose only — the provenance +# below is for whoever edits this file, not for a CLI user: +# +# Hand-authored down to the parameters documented at +# docs.mapbox.com/api/search/places. See `custom-openapi/README.md` for +# how a file like this is wired in. +# +# One command, not two: the API has a single-id GET and a batch POST, and +# an earlier version of this file exposed both as `places get`/`places +# batch`. Reviewed as the wrong shape for any API with that pair — a +# caller should not have to choose between "one id" and "a JSON array of +# ids" written by hand. `batch` is the one that survives, renamed to the +# bare `places` command via `FLATTENED_SERVICES`, taking one or more ids +# as positional arguments (`VARIADIC_BODY_ARRAY` in `src/spec.rs`) and +# always using the batch endpoint — even for a single id — so there is one +# response shape and one code path. The GET operation this used to expose +# as `places get` is gone from here entirely, not just hidden: nothing +# reaches it. +info: + title: "Mapbox Places API" + description: >- + Full detail for one or more places — hours, phone, website, photos, + address, coordinates, activity data — by the `mapbox_id`s a Search Box + API result already returned. This API has no search or suggest of its + own; it only resolves ids something else found. Public Preview, with + a 1000-records-per-account monthly quota. + version: "0.0.0" +servers: + - url: https://api.mapbox.com + description: Places API +paths: + /places/v1/details/retrieve: + post: + operationId: batch + summary: Full detail for one or more places. + description: >- + Each `mapbox_id` comes from a Search Box API result (`search + forward`/`reverse`/`category`) — this API does not look places up + by name or location itself. Always sends a batch request, one id + or many, so a single id reads the same way, no `get`/`batch` + choice to make. Up to 100 ids in one call. + parameters: + - name: "access_token" + in: query + required: true + description: "Mapbox API Access Token" + schema: + type: string + minLength: 1 + requestBody: + required: true + content: + application/json: + schema: + type: object + required: ["ids"] + properties: + ids: + type: array + description: >- + One or more Mapbox IDs, from a Search Box API result, + up to 100. + items: + type: string + minLength: 1 + minItems: 1 + maxItems: 100 + responses: + "200": + description: >- + Every id resolved: `{"results": [, ...]}`, one + entry per id — `name`, `full_address`, `primary_category`/ + `categories`, `coordinates` (with `routable_points`), + structured `address`, `score` (`closed`/`reality`/ + `popularity`, each 0-1), and where available `brand`, + `opening_hours`, `phone`, `photos`, `website`, `building`, and + `telemetry` (hourly activity by day of week). + "206": + description: >- + Partial Content — some ids didn't resolve. Same body as 200, + plus `missing` (ids with no such place) and/or `unprocessed` + (ids that couldn't be processed) alongside `results`. Treated + as success, the same as 200 — a caller still gets every id + that did resolve, and `missing`/`unprocessed` say which ones + didn't rather than failing the whole request over them. + "400": + description: >- + Bad Request — `ids` isn't a non-empty array of at most 100 + entries. The 100 limit is also enforced client-side, before + the request goes out. + "401": + description: Unauthorized + "429": + description: >- + Too Many Requests — the per-second rate limit (charged by id + count) or the Public Preview's monthly quota (1000 + records/account) was exceeded. diff --git a/docs/commands.md b/docs/commands.md index bd1d2e8..38cd59a 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -1,8 +1,8 @@ # Implemented commands -Every command the CLI ships, including five auth commands, 39 API operations (35 across 9 -command groups, plus `directions`, `isochrone`, `map-match` and `matrix`), the -tilesets-cli proxy, `completion` and `generate-skills`. Each is +Every command the CLI ships, including five auth commands, 40 API operations (35 across 9 +command groups, plus `directions`, `isochrone`, `map-match`, `matrix` and +`places`), the tilesets-cli proxy, `completion` and `generate-skills`. Each is shown in both of its renderings. Which one you get is decided by `--output`, whose default (`auto`) reads stdout: a terminal gets the left column, a pipe or redirect gets the right one. See @@ -11,12 +11,12 @@ gets the right one. See Account names, style ids and tokens in the examples are replaced; everything else is as the API sent it. -**35 of the 39 were run against the live API and show what came back:** 24 +**36 of the 40 were run against the live API and show what came back:** 24 on 2026-09-01, `styles download` by 2026-10-01, `fonts list`, `fonts upload` and `fonts delete` on 2026-09-08, once `fonts:list`/`fonts:write` became registrable, -`directions`, `isochrone` and `map-match` on 2026-09-23, `matrix`, -`feedback list` and `feedback get` on 2026-09-24, and `feedback create` on -2026-10-02. +`directions`, `isochrone` and `map-match` on 2026-09-23, `matrix` and +`feedback list`/`feedback get` on 2026-09-24, `feedback create` on +2026-10-02, and `places` on 2026-10-09. The write operations were exercised as round trips on throwaway objects — a style created, updated, drafted and deleted; icons uploaded to a sprite and taken out again; a font uploaded and deleted — leaving the account as @@ -34,9 +34,12 @@ commands and the flags they take — is held to `mapbox --schema` on every can be checked cannot fall behind the binary. The remaining 4 give the response shape from the spec or the docs instead -of a live capture: they're all `search`'s — read-only and safe to run, but -the credentials used to write this page have no Search Box API access, so -every call answers 401 rather than a result. +of a live capture. All four are `search`'s, marked as no-access when this +page was first written — no longer true, discovered while writing +`places`'s example above, which needed a real `search forward` result to +test against and got one. `search`'s own four sections below haven't been +re-captured with this pass, since that's a different command group's page +to touch; a worthwhile follow-up, not done here. Each **Parameters** section lists only what is specific to its command. The globals every API command takes are @@ -103,6 +106,8 @@ nests, and is typed `mapbox styles draft get`. **[Matrix](#matrix)** — [matrix](#mapbox-matrix) +**[Places](#places)** — [places](#mapbox-places) + **[Search](#search)** — [search.forward](#mapbox-search-forward) · [search.reverse](#mapbox-search-reverse) · [search.category](#mapbox-search-category) · @@ -457,11 +462,11 @@ No stored profiles. Run `mapbox auth login` to create one. ## API command groups -35 operations across 9 command groups, plus four single-operation commands -with no group: `directions`, `isochrone`, `map-match` and `matrix`. Seven -groups are generated from the OpenAPI specs vendored in `openapi/`; `search`, -`feedback` and the four single commands are the exceptions — hand-authored -specs versioned in this repo's own `custom-openapi/`, see +35 operations across 9 command groups, plus five single-operation commands +with no group: `directions`, `isochrone`, `map-match`, `matrix` and `places`. +Seven groups are generated from the OpenAPI specs vendored in `openapi/`; +`search`, `feedback` and the five single commands are the exceptions — +hand-authored specs versioned in this repo's own `custom-openapi/`, see `custom-openapi/README.md`. **A command group is not a spec file.** Which command group an operation belongs to is @@ -485,7 +490,7 @@ moved to `tilesets` or `static`. | [`styles`](#styles) | **9** | | [`tilesets`](#tilesets) | **3** | -The [Contents](#contents) list above names every one of the 39. +The [Contents](#contents) list above names every one of the 40. **Everything else the Mapbox specs describe is not here at all.** Not hidden, not shipped as a command that refuses: absent from the spec content @@ -593,8 +598,8 @@ Three things worth knowing about the read forms: A command that changes something takes `--dry-run`, which prints the request it would send, on stdout, and sends nothing. Which commands those are is not -a list anyone keeps: it is every `POST`, `PUT`, `PATCH` and `DELETE` — 13 of -the 39 operations — plus `auth login`, `auth logout`, `auth refresh` and +a list anyone keeps: it is every `POST`, `PUT`, `PATCH` and `DELETE` — 14 of +the 40 operations — plus `auth login`, `auth logout`, `auth refresh` and `generate-skills`. A read-only `GET` does not take it, so `mapbox styles list --dry-run` is a usage error rather than a no-op. It rehearses rather than describes: `--data` is parsed and every `--file` is read, so a @@ -1808,7 +1813,6 @@ is the authority on whether a value is valid, not this page. | `--destinations ` | Which coordinates are matrix columns — same rules as `--sources`. | | `--fallback-speed ` | Replaces a `null` (unreachable) cell with a straight-line estimate at this speed, rather than leaving it `null`. Legacy. | | `--depart-at ` | For future traffic conditions and time-dependent road restrictions. | - #### Examples ```sh @@ -1867,6 +1871,137 @@ Neither output mode has a bespoke rendering for this response, same as --- +## Places + +Full detail for one or more places — hours, phone, website, photos, +address, coordinates, activity data — by the `mapbox_id`s a Search Box +API result already returned. Curated by hand down to the parameters +documented at docs.mapbox.com/api/search/places — see +`custom-openapi/README.md` for why this command group doesn't come from +the vendored specs the way most others do. Public Preview, with a +1000-records-per-account monthly quota. + +**This API has no search or suggest of its own.** It only resolves ids +`search forward`/`reverse`/`category` already returned — the detail-view +follow-up to a search result, not a way to find places by name or location. + +### `mapbox places` + +One or more full place records. No subcommand: the API has a single-id +GET and a batch POST of the same thing, and `mapbox places` always sends +the batch request, one id or many, so there is one command and one +response shape whether given a single id or several. + +#### Parameters + +`...` (positional, one or more, up to 100) — each from a +Search Box API result. More than 100 is a usage error before the request +goes out. + +#### Examples + +```sh +mapbox places dXJuOm1ieHBvaTpmYTE5Y2NhMC0yZmQ3LTQwMzgtYTEzNy02MzFmNGEwZDI5ODA +mapbox places dXJuOm1ieHBvaTpmYTE5Y2NhMC0yZmQ3LTQwMzgtYTEzNy02MzFmNGEwZDI5ODA \ + dXJuOm1ieHBvaTo1NTM4ZTE2OC0yMDM1LTRjODItODBkOC01NjY2YWM0MzhkYmE +``` + +#### Outputs + +The response is always `{"results": [, ...]}`, one entry +per id, each carrying `name`, `full_address`, `primary_category`/ +`categories`, `coordinates` (with `routable_points`), structured +`address`, `score` (`closed`/`reality`/`popularity`, each 0-1), and where +available `brand`, `opening_hours`, `phone`, `photos`, `website`, +`building`, and `telemetry` (hourly activity by day of week). + +Captured live, a real place found via `search forward --q "Ferry Building +San Francisco"`: + + + + +
Terminal — -o textAgent — -o json
+ +```text +NAME BRAND CREATED_AT FULL_ADDRESS MAPBOX_ID OPENING_HOURS PERMANENTLY_CLOSED PHONE PRIMARY_CATEGORY STATUS UPDATED_AT WEBSITE +Ferry Building - 2026-07-0… San Francis… dXJuOm1i… Sa 08:00-14:… no +141523… food active 2026-10-0… http://… +``` + + + +```json +{"results":[{"address":{"city":"San Francisco","neighborhood":"Financial District","postcode":"94105","region":"California","region_code_full":"US-CA","country":"United States","country_code":"US"},"attributes":{"accommodation_wheelchair_accessible_entrance":true,"offering_coffee":true,"offering_dessert":true,"offering_lunch":true,"service_dine_in":true,"service_takeout":true},"brand":null,"categories":["cafe","food","food_and_drink"],"coordinates":{"latitude":37.79557765,"longitude":-122.39332918,"source":"poi","routable_points":[{"name":"driving","latitude":37.79559358806043,"longitude":-122.39333830635825}]},"created_at":"2026-07-02T02:56:22.965","full_address":"San Francisco, California, 94105, United States","mapbox_id":"dXJuOm1ieHBvaTpmYTE5Y2NhMC0yZmQ3LTQwMzgtYTEzNy02MzFmNGEwZDI5ODA","name":"Ferry Building","opening_hours":"Sa 08:00-14:00","permanently_closed":false,"phone":"+14152373318","primary_category":"food","score":{"closed":0,"reality":0.973,"popularity":0.275},"status":"active","updated_at":"2026-10-06T04:01:42.871","website":"http://crumbleandwhisk.com/"}]} +``` + +
+ +Dropped most of `attributes` (14+ boolean amenity flags — wheelchair +access, payment types, and the like) for length; the real response +carries them all. `brand` is `null` here since this isn't a chain +location. + +A second id adds a second row to the same table — captured live by +pairing the Ferry Building id above with a real Starbucks location +(`search forward --q "Starbucks" --proximity …`): + + + + +
Terminal — -o textAgent — -o json
+ +```text +NAME BRAND FULL_ADDRESS MAPBOX_ID OPENING_HOURS PHONE PRIMARY_CATEGORY WEBSITE +Ferry Building - San Francis… dXJuOm1i… Sa 08:00-14:… +141523… food http://… +Starbucks Starbuc… 7 Drumm Str… dXJuOm1i… Mo 05:30-13:… +134138… teahouse https:/… +``` + + + +```json +{"results":[{"mapbox_id":"dXJuOm1ieHBvaTpmYTE5Y2NhMC0yZmQ3LTQwMzgtYTEzNy02MzFmNGEwZDI5ODA","name":"Ferry Building","brand":null,"full_address":"San Francisco, California, 94105, United States","primary_category":"food","status":"active","opening_hours":"Sa 08:00-14:00","phone":"+14152373318","website":"http://crumbleandwhisk.com/"},{"mapbox_id":"dXJuOm1ieHBvaTo1NTM4ZTE2OC0yMDM1LTRjODItODBkOC01NjY2YWM0MzhkYmE","name":"Starbucks","brand":"Starbucks","full_address":"7 Drumm Street, San Francisco, California 94111, United States","primary_category":"teahouse","status":"active","opening_hours":"Mo 05:30-13:00; Tu 05:30-13:00; We 05:30-13:00; Th 05:30-13:00; Fr 05:30-13:00","phone":"+13413880001","website":"https://www.starbucks.com/store-locator/store/1013857/"}]} +``` + +
+ +The response's own nested objects (`coordinates`, `address`, `score`, +`attributes`) and arrays (`categories`, `photos`) are dropped from the +table — only scalar fields become columns, the same rule every table on +this page follows — so the full detail is a `-o json` away. `CREATED_AT`, +`PERMANENTLY_CLOSED`, `STATUS` and `UPDATED_AT` drop out of the two-row +table above for a different reason: every row shares the same value for +each (this dataset's own coincidence, not something the API promises), +and a column that doesn't vary across rows is dropped as uninformative — +the same "varying" rule the rest of this page's tables follow. + +`206` with `missing` and/or `unprocessed` alongside `results` — some ids +didn't resolve — is still exit 0: the caller got every id that did +resolve. Captured live by pairing the Ferry Building id above with a +well-formed id that doesn't exist: + +```json +{"missing":["dXJuOm1ieHBvaTo2NTNkZjMzMS0zNTA2LTQ1MGEtYjg2Yi0wODY0ZmQ2NDRiYzA"],"results":[{"mapbox_id":"dXJuOm1ieHBvaTpmYTE5Y2NhMC0yZmQ3LTQwMzgtYTEzNy02MzFmNGEwZDI5ODA","name":"Ferry Building","full_address":"San Francisco, California, 94105, United States","primary_category":"food","status":"active","opening_hours":"Sa 08:00-14:00","phone":"+14152373318","website":"http://crumbleandwhisk.com/"}]} +``` + +Two keys at the top level here, not one, so `-o text` skips the usual +one-key-object-to-table unwrap and falls back to pretty-printed JSON — +the same response, read either way. + +When *no* id resolves at all, the CLI exits non-zero instead of reporting +success with an empty `results` — the old `places get` answered that case +with a 404, and a silent exit 0 here would be a regression from that. The +ids that didn't resolve are printed to stderr: + +```console +$ mapbox places dXJuOm1ieHBvaTo2NTNkZjMzMS0zNTA2LTQ1MGEtYjg2Yi0wODY0ZmQ2NDRiYzA +No id resolved: dXJuOm1ieHBvaTo2NTNkZjMzMS0zNTA2LTQ1MGEtYjg2Yi0wODY0ZmQ2NDRiYzA +Error: No id resolved. See the ids above, or re-check them against a Search Box result. +$ echo $? +1 +``` + +--- + ## Search The public, non-interactive surface of the Search Box API: text search, diff --git a/src/executor.rs b/src/executor.rs index 14b1469..6664141 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -11,7 +11,8 @@ use crate::link; use crate::output::{self, CliError, Mode}; use crate::remedy::{self, Remedy}; use crate::spec::{ - Operation, Parameter, RequestBody, ACCOUNT_PLACEHOLDERS, MULTIPART, UNESCAPED_PATH_PARAMS, + Operation, Parameter, RequestBody, VariadicBodyArray, ACCOUNT_PLACEHOLDERS, MULTIPART, + UNESCAPED_PATH_PARAMS, }; /// The query parameter the access token travels in, and what stands in for @@ -212,7 +213,8 @@ fn dispatch( .as_ref() .map(|argument| argument.body.as_ref()); let merged_body = merge_body_fields(&op.body_fields, matches, data)?; - let data = merged_body.as_deref().or(data); + let variadic_body = variadic_body_array_json(&op.variadic_body_array, matches); + let data = variadic_body.as_deref().or(merged_body.as_deref()).or(data); let files: Vec<&str> = matches .try_get_many::("file") .ok() @@ -363,25 +365,28 @@ fn dispatch( match as_text { Some(text) => match serde_json::from_str::(&text) { - Ok(json) => match matches.get_one::(output::FILTER_ARG) { - Some(wanted) => output::emit_value( - mode, - &output::pick_row(&json, wanted) - .map_err(|err| with_page_context(err, next_page.as_ref()))?, - None, - Some(&op.service), - // The wanted row is already in hand, so no need for a - // pagination tip about the other rows. - None, - )?, - None => output::emit_value( - mode, - &json, - detail_hint(op).as_deref(), - Some(&op.service), - next_page.as_ref().map(NextPage::tip).as_deref(), - )?, - }, + Ok(json) => { + no_place_resolved(op, &json)?; + match matches.get_one::(output::FILTER_ARG) { + Some(wanted) => output::emit_value( + mode, + &output::pick_row(&json, wanted) + .map_err(|err| with_page_context(err, next_page.as_ref()))?, + None, + Some(&op.service), + // The wanted row is already in hand, so no need for a + // pagination tip about the other rows. + None, + )?, + None => output::emit_value( + mode, + &json, + detail_hint(op).as_deref(), + Some(&op.service), + next_page.as_ref().map(NextPage::tip).as_deref(), + )?, + } + } Err(_) => output::emit_text_body(mode, &text)?, }, // Bytes bypass the output contract entirely. A PNG can't be @@ -976,6 +981,25 @@ fn read_stdin() -> Result { Ok(body) } +/// The JSON body a [`VariadicBodyArray`] positional builds on its own +/// — `{"": [, ...]}` — or `None` when the operation has none, +/// the ordinary `--data`/body-field-flags path applies instead. `--data` is +/// never registered for such an operation (see `main.rs`), so there is +/// nothing to merge with here, unlike [`merge_body_fields`]. +fn variadic_body_array_json( + variadic: &Option, + matches: &ArgMatches, +) -> Option { + let variadic = variadic.as_ref()?; + let values: Vec<&str> = matches + .get_many::(&variadic.arg_name) + .into_iter() + .flatten() + .map(String::as_str) + .collect(); + Some(serde_json::json!({ variadic.field.clone(): values }).to_string()) +} + /// `--data` with the body-field flags set on it, or `None` if nothing was /// set. A flag wins over `--data`; a default fills only what neither set. fn merge_body_fields( @@ -1203,6 +1227,40 @@ fn part_media_type(path: &str) -> &'static str { } } +/// `mapbox places`'s 200/206 is "success" at the HTTP level even when not +/// one id resolved — `results` empty, every id in `missing`/`unprocessed`. +/// The old `places get` answered that case with a 404; exiting 0 here with +/// nothing on stderr would be a silent regression from that, since a script +/// or agent checking the exit code would read "fetched zero records" the +/// same as "fetched the one record asked for". Partial success (some ids +/// resolved, some didn't) still exits 0 — the caller got something real. +fn no_place_resolved(op: &Operation, json: &serde_json::Value) -> Result<()> { + if op.service != "places" { + return Ok(()); + } + let Some(results) = json["results"].as_array() else { + return Ok(()); + }; + if !results.is_empty() { + return Ok(()); + } + let unresolved: Vec<&str> = ["missing", "unprocessed"] + .into_iter() + .filter_map(|key| json[key].as_array()) + .flatten() + .filter_map(|id| id.as_str()) + .collect(); + if !unresolved.is_empty() { + output::progress(&format!("No id resolved: {}", unresolved.join(", "))); + } + Err(CliError::new( + "no_results", + "No id resolved. See the ids above, or re-check them against a Search Box result." + .to_string(), + ) + .into()) +} + /// What to say when a successful response carries no body. /// /// The HTTP method is all we have to go on: generated commands share one @@ -1531,10 +1589,10 @@ mod tests { use super::{ binary_notice, binary_notice_enabled, describe_body, empty_success_line, extra_query_from_env, file_name_of, is_binary_content_type, merge_body_fields, - part_media_type, path_segment, path_segment_for, payload_of, query_pairs, redacted_url, - request_id, resolve_body_source, resolve_data, shell_value, substitute_path_param, - with_page_context, BodySource, NextPage, ResponseHeaders, ACCESS_TOKEN, EXTRA_QUERY_ENV, - REQUEST_ID_HEADERS, + no_place_resolved, part_media_type, path_segment, path_segment_for, payload_of, + query_pairs, redacted_url, request_id, resolve_body_source, resolve_data, shell_value, + substitute_path_param, with_page_context, BodySource, NextPage, ResponseHeaders, + ACCESS_TOKEN, EXTRA_QUERY_ENV, REQUEST_ID_HEADERS, }; use std::borrow::Cow; @@ -2761,4 +2819,53 @@ mod tests { path_segment_for("styles", &style_id, "../../tokens/v2/victim").expect("encoded"); assert_eq!(safe, "..%2F..%2Ftokens%2Fv2%2Fvictim"); } + + fn places_operation() -> crate::spec::Operation { + crate::spec::effective_services() + .expect("the bundled specs parse") + .into_iter() + .find(|svc| svc.name == "places") + .expect("places is a real service") + .operations + .into_iter() + .next() + .expect("places has its one operation") + } + + /// Some ids missing is a real, partial success — the caller gets every + /// id that did resolve, so this must not refuse. + #[test] + fn a_partial_batch_with_some_results_is_not_refused() { + let op = places_operation(); + let json = serde_json::json!({"results": [{"mapbox_id": "a"}], "missing": ["b"]}); + assert!(no_place_resolved(&op, &json).is_ok()); + } + + /// Every id missing is the old `places get`'s 404 in different clothes — + /// exiting 0 here would be a silent regression from that. + #[test] + fn a_batch_with_no_results_at_all_is_refused() { + let op = places_operation(); + let json = serde_json::json!({"results": [], "missing": ["a", "b"]}); + let err = refusal(no_place_resolved(&op, &json).unwrap_err()); + assert_eq!(err.code, "no_results"); + } + + /// Not just places' own shape of "nothing resolved" — a response this + /// check doesn't recognize at all must not be mistaken for one. + #[test] + fn a_response_with_no_results_field_is_not_refused() { + let op = places_operation(); + assert!(no_place_resolved(&op, &serde_json::json!({"ok": true})).is_ok()); + } + + /// The whole check is places-specific: another service's response + /// might coincidentally have an empty `results` array and mean nothing + /// by it. + #[test] + fn another_service_with_an_empty_results_array_is_not_refused() { + let mut op = places_operation(); + op.service = "search".to_string(); + assert!(no_place_resolved(&op, &serde_json::json!({"results": []})).is_ok()); + } } diff --git a/src/help_layout.rs b/src/help_layout.rs index d50f7c1..3561fba 100644 --- a/src/help_layout.rs +++ b/src/help_layout.rs @@ -35,7 +35,7 @@ const GROUPS: &[(&str, &[&str])] = &[ "Maps and data", &["styles", "sprites", "fonts", "tilesets", "static"], ), - ("Search", &["search", "geocoder", "feedback"]), + ("Search", &["search", "geocoder", "places", "feedback"]), ( "Navigation", &["directions", "matrix", "isochrone", "map-match"], @@ -75,6 +75,10 @@ const DESCRIPTIONS: &[(&str, &str)] = &[ "Find addresses and places by text, coordinate or category", ), ("geocoder", "Forward, reverse and batch geocoding"), + ( + "places", + "Full detail for a place, by its Search Box result id", + ), ("feedback", "Submit and list feedback about Mapbox data"), ("directions", "Route between waypoints"), ( diff --git a/src/main.rs b/src/main.rs index f3f0959..dc87e78 100644 --- a/src/main.rs +++ b/src/main.rs @@ -283,7 +283,11 @@ fn build_operation_command(op: &spec::Operation) -> Command { // `--data` rather than `--file` — `starFile`'s whole body is the word // `true`. let text_body = body.text_content_type(); - let takes_data = body.accepts_json() || text_body.is_some(); + // A variadic positional builds the whole body on its own — see + // below — so `--data` would be a second, redundant way to say the + // same thing, with nothing else the body ever carries. + let takes_data = + (body.accepts_json() || text_body.is_some()) && op.variadic_body_array.is_none(); if takes_data { cmd = cmd.arg( Arg::new("data") @@ -345,6 +349,21 @@ fn build_operation_command(op: &spec::Operation) -> Command { cmd = cmd.arg(arg); } + if let Some(variadic) = &op.variadic_body_array { + let help = variadic + .description + .as_deref() + .map(first_sentence) + .unwrap_or_default() + .to_string(); + cmd = cmd.arg( + Arg::new(variadic.arg_name.clone()) + .required(true) + .num_args(1..=variadic.max) + .help(help), + ); + } + // Only where it can mean something. A `GET` has no plan worth previewing, // and the same reasoning that keeps unusable operations off the command // surface entirely keeps this flag off the commands it would do nothing diff --git a/src/remedy.rs b/src/remedy.rs index 2a0476c..578023d 100644 --- a/src/remedy.rs +++ b/src/remedy.rs @@ -94,6 +94,7 @@ const SERVICE_DOCS: &[(&str, &str)] = &[ "https://docs.mapbox.com/api/navigation/map-matching/", ), ("matrix", "https://docs.mapbox.com/api/navigation/matrix/"), + ("places", "https://docs.mapbox.com/api/search/places/"), ("search", "https://docs.mapbox.com/api/search/search-box/"), // Static Images and Static Tiles merged into one `static` command group // (#116); neither upstream page covers both, so this points at Static diff --git a/src/schema.rs b/src/schema.rs index 63cf463..6e1edf8 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -474,7 +474,14 @@ fn api_command(app: &Command, svc: &ServiceSpec, op: &Operation) -> CommandEntry arguments.extend( op.body .as_ref() - .map(|body| body_arguments(body, !op.body_fields.is_empty(), declared)) + .map(|body| { + body_arguments( + body, + !op.body_fields.is_empty(), + op.variadic_body_array.is_some(), + declared, + ) + }) .unwrap_or_default(), ); @@ -484,6 +491,14 @@ fn api_command(app: &Command, svc: &ServiceSpec, op: &Operation) -> CommandEntry ..parameter(field, ArgKind::Option) })); + arguments.extend(op.variadic_body_array.as_ref().map(|variadic| Argument { + required: true, + repeatable: true, + location: Some("body"), + description: variadic.description.clone(), + ..Argument::new(variadic.arg_name.clone(), ArgKind::Positional) + })); + // Described because it is declared — `the_schema_lists_the_flags_clap_declares` // holds the two to each other, and a flag missing here is a flag an agent // has no way to learn about. It has no `location`: it is the one argument @@ -637,10 +652,11 @@ fn username_argument(op: &Operation) -> Option { fn body_arguments( body: &RequestBody, has_field_flags: bool, + suppress_data: bool, declared: Option<&Command>, ) -> Vec { let text = body.text_content_type(); - let takes_data = body.accepts_json() || text.is_some(); + let takes_data = (body.accepts_json() || text.is_some()) && !suppress_data; let file_type = body.file_content_type(); // `starFile` and `initUpload` accept either flag for the same body. When @@ -927,9 +943,12 @@ mod tests { .filter(|arg| arg.source.is_none()) .map(|arg| { let spelling = arg.flag.clone().unwrap_or_else(|| arg.name.clone()); - let required = if arg.location == Some("body") { + let required = if arg.location == Some("body") && arg.kind != ArgKind::Positional { // Whatever clap says, so this comparison stays about the - // fields it can speak to. + // fields it can speak to. A `VariadicBodyArray` + // positional is exempt: unlike `--data`/`--file`, clap + // has no either-or ambiguity to paper over for it, its + // `required` is exactly what `main.rs` declared. false } else { arg.required diff --git a/src/spec.rs b/src/spec.rs index 0d93fed..c3bc8ec 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -30,6 +30,9 @@ pub struct Operation { pub body: Option, /// JSON body properties that are also flags. See [`BODY_FIELD_FLAGS`]. pub body_fields: Vec, + /// A single JSON body array field exposed as one or more positional + /// arguments instead of `--data`. See [`VARIADIC_BODY_ARRAY`]. + pub variadic_body_array: Option, pub base_url: String, /// Set when the operation needs a scope that isn't registrable as OAuth /// (see `UNSUPPORTED_OPERATIONS` below). A `mapbox auth login` token can @@ -91,6 +94,69 @@ const BODY_FIELD_FLAGS: &[(&str, &str, &[BodyField])] = &[( ], )]; +/// (service, operationId, body field name, CLI positional name, max count) +/// for an operation whose JSON body takes a single array field, exposed as +/// one or more positional arguments collected into it instead of `--data`. +/// +/// The rule for any API shaped like a single-item GET and a batch POST of +/// the same thing: one command, not a choice between "one id" and a +/// hand-typed JSON array. The single-item operation is dropped from the +/// spec entirely rather than kept and hidden — see `places.yaml`'s own +/// header for the first case this applied to. +const VARIADIC_BODY_ARRAY: &[(&str, &str, &str, &str, usize)] = + &[("places", "batch", "ids", "mapbox-id", 100)]; + +/// A positional argument collected into one JSON array body field, in +/// place of `--data`. See [`VARIADIC_BODY_ARRAY`]. +#[derive(Debug, Clone)] +pub struct VariadicBodyArray { + pub field: String, + pub arg_name: String, + pub max: usize, + pub description: Option, +} + +fn variadic_body_array_names( + service: &str, + operation_id: &str, +) -> Option<(&'static str, &'static str, usize)> { + VARIADIC_BODY_ARRAY + .iter() + .find(|(s, id, _, _, _)| *s == service && *id == operation_id) + .map(|(_, _, field, arg_name, max)| (*field, *arg_name, *max)) +} + +/// Builds the [`VariadicBodyArray`] for an operation [`variadic_body_array_names`] +/// names, reading the field's own description off the request body schema +/// the same way [`body_field_parameters`] does for a scalar field. +fn variadic_body_array_parameter( + op: &Value, + full_spec: &Value, + field: &str, + arg_name: &str, + max: usize, +) -> Result { + let schema = &op["requestBody"]["content"]["application/json"]["schema"]; + let schema = match schema["$ref"].as_str() { + Some(reference) => resolve_ref(full_spec, reference).unwrap_or(schema), + None => schema, + }; + let property = &schema["properties"][field]; + anyhow::ensure!( + property["type"].as_str() == Some("array") + && property["items"]["type"].as_str() == Some("string"), + "body property `{field}` is listed in VARIADIC_BODY_ARRAY but is not an array of \ + strings in the spec" + ); + let description = property["description"].as_str().map(first_paragraph); + Ok(VariadicBodyArray { + field: field.to_string(), + arg_name: arg_name.to_string(), + max, + description, + }) +} + /// (service, operationId, the media type the API actually wants) for /// operations whose spec gets the content type wrong. /// @@ -172,7 +238,8 @@ fn arg_name_override(service_name: &str, param_name: &str) -> Option<&'static st /// `generate-skills`, this file's own `command()` above — reads a /// [`FLATTENED_SERVICES`] service correctly for free, because they all go /// through `command()` rather than reconstructing the string themselves. -pub const FLATTENED_SERVICES: &[&str] = &["directions", "isochrone", "map-match", "matrix"]; +pub const FLATTENED_SERVICES: &[&str] = + &["directions", "isochrone", "map-match", "matrix", "places"]; /// (service, path parameter name) pairs whose value is trusted to reach the /// URL unescaped, because every legitimate value already contains a @@ -756,6 +823,10 @@ pub const CUSTOM_SPEC_ENTRIES: &[SpecEntry] = &[ name: "matrix", yaml: include_str!("../custom-openapi/matrix/openapi/matrix.yaml"), }, + SpecEntry { + name: "places", + yaml: include_str!("../custom-openapi/places/openapi/places.yaml"), + }, ]; /// The list the CLI actually generates commands from: [`MAPBOX_SPEC_ENTRIES`], @@ -1088,6 +1159,17 @@ pub fn parse_spec(service_name: &str, yaml: &str) -> Result { None => vec![], }; + let variadic_body_array = match operation_id + .and_then(|id| variadic_body_array_names(service_name, id)) + { + Some((field, arg_name, max)) => Some( + variadic_body_array_parameter(op, &doc, field, arg_name, max).with_context( + || format!("{service_name} {}", operation_id.unwrap_or("")), + )?, + ), + None => None, + }; + let disabled_scope = operation_id.and_then(|id| unsupported_scope_for(service_name, id)); @@ -1106,6 +1188,7 @@ pub fn parse_spec(service_name: &str, yaml: &str) -> Result { query_params, body, body_fields, + variadic_body_array, base_url: base_url.clone(), disabled_scope, detail: None, @@ -1914,6 +1997,63 @@ requestBody: assert_eq!(fields[0].description.as_deref(), Some("Latitude.")); } + #[test] + fn a_variadic_body_array_needs_an_array_of_strings_the_spec_declares() { + let op: Value = serde_yaml::from_str( + r#" +requestBody: + content: + application/json: + schema: + type: object + required: [ids] + properties: + ids: { type: array, description: "One or more ids.", items: { type: string } } + names: { type: array, items: { type: number } } +"#, + ) + .unwrap(); + + let variadic = variadic_body_array_parameter(&op, &op, "ids", "mapbox-id", 100).unwrap(); + assert_eq!(variadic.field, "ids"); + assert_eq!(variadic.arg_name, "mapbox-id"); + assert_eq!(variadic.max, 100); + assert_eq!(variadic.description.as_deref(), Some("One or more ids.")); + + assert!(variadic_body_array_parameter(&op, &op, "names", "name", 100).is_err()); + assert!(variadic_body_array_parameter(&op, &op, "missing", "x", 100).is_err()); + } + + /// `places` is the first (and so far only) `VARIADIC_BODY_ARRAY` / + /// `FLATTENED_SERVICES` service: one operation, no subcommand, its one + /// positional feeding the batch endpoint's `ids` array instead of + /// `--data`, even for a single id. + #[test] + fn places_is_flattened_and_variadic() { + let spec = parse_spec( + "places", + include_str!("../custom-openapi/places/openapi/places.yaml"), + ) + .expect("places.yaml parses"); + + assert!(FLATTENED_SERVICES.contains(&"places")); + let batch = spec + .operations + .iter() + .find(|op| op.command_path == ["batch"]) + .expect("the batch operation exists in the spec"); + assert!(batch.is_exposed()); + assert_eq!(batch.command(), "places"); + assert!(batch.body_fields.is_empty()); + let variadic = batch + .variadic_body_array + .as_ref() + .expect("batch has a variadic_body_array"); + assert_eq!(variadic.field, "ids"); + assert_eq!(variadic.arg_name, "mapbox-id"); + assert_eq!(variadic.max, 100); + } + #[test] fn an_operation_with_no_request_body_has_none() { let svc = service(BODIES); diff --git a/tests/dry_run.rs b/tests/dry_run.rs index 0f8ca90..58e28e1 100644 --- a/tests/dry_run.rs +++ b/tests/dry_run.rs @@ -438,3 +438,72 @@ fn the_plan_is_a_result_and_goes_to_stdout() { "{text}" ); } + +/// `places` always sends the batch request, one id or many — the whole +/// point of merging `places get`/`places batch` into one command. +#[test] +fn places_always_batches_even_for_one_id() { + let config = config_dir("places-one-id"); + let out = run( + &config, + &[ + "-o", + "json", + "--token", + "sk.a-test-token", + "places", + "zz-mapbox-id-1", + "--dry-run", + ], + ); + + assert!(out.status.success(), "{}", stderr(&out)); + let body = &json(&stdout(&out))["body"]["json"]; + assert_eq!(body, &serde_json::json!({ "ids": ["zz-mapbox-id-1"] })); +} + +#[test] +fn places_takes_more_than_one_id() { + let config = config_dir("places-many-ids"); + let out = run( + &config, + &[ + "-o", + "json", + "--token", + "sk.a-test-token", + "places", + "zz-mapbox-id-1", + "zz-mapbox-id-2", + "zz-mapbox-id-3", + "--dry-run", + ], + ); + + assert!(out.status.success(), "{}", stderr(&out)); + let body = &json(&stdout(&out))["body"]["json"]; + assert_eq!( + body, + &serde_json::json!({ "ids": ["zz-mapbox-id-1", "zz-mapbox-id-2", "zz-mapbox-id-3"] }) + ); +} + +/// The API's own limit (100), enforced before the request goes out rather +/// than left to a 400 that says the same thing less directly. +#[test] +fn places_refuses_more_than_a_hundred_ids_before_sending() { + let config = config_dir("places-too-many-ids"); + let ids: Vec = (0..101).map(|n| format!("zz-id-{n}")).collect(); + let mut args = vec!["-o", "json", "--token", "sk.a-test-token", "places"]; + args.extend(ids.iter().map(String::as_str)); + args.push("--dry-run"); + + let out = run(&config, &args); + + assert_eq!(out.status.code(), Some(2), "{}", stderr(&out)); + assert!( + stderr(&out).contains("no more were expected"), + "{}", + stderr(&out) + ); +} diff --git a/tests/fixtures/api_command_surface.txt b/tests/fixtures/api_command_surface.txt index 2c9b3b4..d2bc796 100644 --- a/tests/fixtures/api_command_surface.txt +++ b/tests/fixtures/api_command_surface.txt @@ -14,6 +14,7 @@ mapbox geocoder reverse | aliases: (none) mapbox isochrone | aliases: (none) mapbox map-match | aliases: (none) mapbox matrix | aliases: (none) +mapbox places | aliases: (none) mapbox search category | aliases: (none) mapbox search forward | aliases: (none) mapbox search list-category | aliases: (none)