From 23c660d2f92d7b93f95277dbbf44ef3192026ab6 Mon Sep 17 00:00:00 2001 From: Andrew Bryant Date: Fri, 23 Oct 2026 18:18:12 -0400 Subject: [PATCH] Add retry with backoff, and narrow the untyped responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Retry: - `RetryPolicy` (3 retries, full-jitter exponential backoff) applied in a new `execute` dispatch chokepoint, configurable via `with_retry_policy`. `Retry-After` is honoured, and one longer than `max_delay` is surfaced rather than slept through. - What may be replayed depends on the method, not just the status. A 429 means the server refused the request without processing it, so any method is safe. A 5xx or timeout gives no such guarantee, so only idempotent methods are retried: this is a billing API, and replaying a POST the server had already applied would duplicate an invoice, payment, or email. Response types: - `statics`, `search`, and `health_check` return `Statics`, `SearchResults`, and `HealthCheck` instead of being generic. `HealthCheck` covers both deployments — hosted returns a bare message rather than a report — and `SearchResults` covers both the Elasticsearch and non-Elasticsearch bucket sets. - Each keeps the `extra` convention, and `statics_as` / `search_as` / `health_check_as` remain for pinning shapes left untyped. Nested config blobs (gateways, templates, extensions) stay `Value`, same reasoning as `Company::settings`. - `send_email` previously decoded the response as a bare body, but the controller returns `itemResponse(...)` — a `data`-wrapped entity. It now unwraps, so `T` is the entity. Still generic, since the endpoint serves invoices, quotes, credits, and purchase orders. - `chart` stays generic on purpose: the eleven charts share no response shape. - Adds `id_as_string`, since `/statics` returns `payment_types.id` as a string and the other tables return numbers. Adds CHANGELOG.md (Keep a Changelog), and a CLAUDE.md note to keep it current. 17 new tests, 43 in client_requests.rs total. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 77 +++++++ CLAUDE.md | 28 ++- COVERAGE.md | 26 ++- Cargo.toml | 3 + README.md | 16 +- src/client.rs | 199 ++++++++++++++---- src/lib.rs | 4 +- src/models/common.rs | 22 ++ src/models/health.rs | 79 +++++++ src/models/mod.rs | 6 + src/models/search.rs | 65 ++++++ src/models/statics.rs | 112 ++++++++++ src/retry.rs | 149 +++++++++++++ tests/client_requests.rs | 444 +++++++++++++++++++++++++++++++++++++++ 14 files changed, 1179 insertions(+), 51 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 src/models/health.rs create mode 100644 src/models/search.rs create mode 100644 src/models/statics.rs create mode 100644 src/retry.rs diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..e0bd805 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,77 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [Unreleased] + +### Added + +- Wrappers for every one of the API's 34 `Route::resource` groups, plus + `users` — 265 methods. See `COVERAGE.md` for the per-endpoint inventory. +- `COVERAGE.md`, a detailed record of what is wrapped, what is not, and why, + checked against `routes/api.php` on the `v5-stable` branch. +- Retry with full-jitter exponential backoff, configurable through + `RetryPolicy` and `InvoiceNinjaClient::with_retry_policy`. Defaults to 3 + retries. `Retry-After` is honoured when the server sends it. +- `InvoiceNinjaClient::with_timeout` and a `DEFAULT_TIMEOUT` of 60 seconds + applied to every request. +- Typed responses for previously untyped endpoints: `Statics`, `StaticItem`, + `Currency`, `DateFormat`, `SearchResults`, `SearchHit`, `HealthCheck`, and + `PhpVersion`. +- `statics_as`, `search_as`, and `health_check_as` as generic escape hatches + for callers who need to pin a shape the typed models leave in `extra`. +- `tests/client_requests.rs`: 43 tests covering the request plumbing against + a loopback `wiremock` server. No network or credentials required. + +### Changed + +- `statics`, `search`, and `health_check` return concrete types instead of + being generic over `T: DeserializeOwned`. +- `send_email` now unwraps the `data` envelope, so `T` is the entity + (`Invoice`, `Quote`, …) rather than a wrapper the caller had to model. It + stays generic because the endpoint serves all four entity types. +- `chart` remains generic, deliberately: the eleven chart endpoints share no + common response shape, so there is nothing to narrow to. +- The base URL is parsed and validated when the client is constructed. A + missing scheme used to produce a 404 on every call instead of one error. +- `list_all_*` terminates on `meta.pagination.total_pages` when the server + sends it, falling back to the short-page heuristic only when it does not. + +### Fixed + +- `list_all_*` silently truncated results. The server caps page size at + `min(abs(per_page), 5000)`, so a page shorter than requested did not mean + the last page — the walk stopped early and returned a partial list as + though it were complete. `per_page` is now clamped to the server maximum so + the fallback heuristic holds. +- `list_all_*` with `per_page(0)` never terminated: the check `got < 0` has + no solution. It now falls back to the default page size, and a `MAX_PAGES` + backstop raises `Error::PaginationLimit` rather than quietly truncating. +- Requests could hang indefinitely. `reqwest` applies no timeout of its own + and none was configured. +- Ids were interpolated into request paths unescaped, so a `/`, `?`, or `#` + in an id changed which endpoint was addressed. + +### Security + +- Ids used as URL path segments are escaped, and ids that are empty or + consist only of dots are rejected. Escaping alone is insufficient: the + WHATWG rules the `url` crate implements decode `%2E` back to `.` before + resolving dot segments, so an escaped `..` still collapsed the path. An + empty id would have addressed the collection route, making + `delete_client("")` mean `DELETE /clients`. +- `POST` is never replayed on a 5xx or timeout, since the server may have + applied it before the response was lost and a replay would duplicate an + invoice, payment, or email. A 429 is the exception — the server states it + refused the request, so no work was done. + +## [0.1.0] - 2026-10-23 + +### Added + +- Initial release: async client for the Invoice Ninja v5 API covering + clients, invoices, quotes, credits, payments, products, expenses, + projects, tasks, and vendors. diff --git a/CLAUDE.md b/CLAUDE.md index be83e02..723390c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -34,6 +34,17 @@ serde layer, and `client_requests.rs` covers the request plumbing (pagination, path escaping, headers, timeouts, error mapping) against a `wiremock` server on loopback — no credentials or internet needed. +## Changelog + +`CHANGELOG.md` follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) +and semantic versioning. **Update it in the same change that alters +behaviour** — add an entry under `## [Unreleased]`, in the right section +(`Added`, `Changed`, `Deprecated`, `Removed`, `Fixed`, `Security`), creating +the section if it is not there yet. Entries describe the effect on a user of +the crate, not the implementation: say what broke or changed for a caller and +why, not which function was edited. Purely internal work (refactors, test-only +changes, doc tweaks) does not need an entry. + ## Architecture - `src/client.rs` — `InvoiceNinjaClient` holds a `reqwest::Client` (with the @@ -60,11 +71,18 @@ serde layer, and `client_requests.rs` covers the request plumbing `/bulk` route** — currently `client_gateway_tokens`, `companies`, and `system_logs`. Check `routes/api.php` before assuming a resource has one; a generated `bulk_*` against a missing route just 405s. -- Every request is built through the private `req(method, path)` helper, - which is the single place the optional `X-API-PASSWORD` header and the - per-request timeout are attached. **Don't call `self.http` directly in a - new helper** — it would silently skip both. Only `req` itself should touch - `self.http`. +- Requests go through two chokepoints: `req(method, path)` builds them and is + the single place the optional `X-API-PASSWORD` header and the per-request + timeout are attached, and `execute(builder)` dispatches them and is the + single place retrying happens. **A new helper must use both** — calling + `self.http` directly would skip the header, the timeout, and the retry + policy. Only `req` and `execute` should touch `self.http`. +- `src/retry.rs` — `RetryPolicy` plus the rules for what may be replayed. + **The method matters, not just the status.** A 429 means the server + refused the request without processing it, so any method is safe to + retry; a 5xx or timeout gives no such guarantee, so only idempotent + methods are. **Don't extend retrying to `POST` on 5xx** — this is a + billing API, and a replayed create would duplicate an invoice or payment. - **Interpolate ids into paths with `seg(id)?`, never raw.** It escapes the value into a single path segment and rejects ids that are empty or dot-only. Both halves matter: escaping stops a `/` or `?` in an id from diff --git a/COVERAGE.md b/COVERAGE.md index f3b6858..4f25f1c 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -242,10 +242,28 @@ written yet: ### Typed response gaps -`statics`, `chart`, `search`, `send_email`, and `health_check` are generic -over `T: DeserializeOwned` rather than returning concrete structs, because -none of them has a stable published schema. Pass `serde_json::Value`, or your -own struct to pin the parts you rely on. +`statics`, `search`, and `health_check` return concrete types (`Statics`, +`SearchResults`, `HealthCheck`). Each keeps the crate's `extra` convention, +so fields they don't model stay reachable rather than being dropped, and +each has a generic counterpart — `statics_as`, `search_as`, +`health_check_as` — for pinning a shape they leave untyped. + +Two remain generic on purpose: + +- **`chart`** — the eleven chart endpoints share no common response shape + (`totals_v2` returns per-currency maps, `cashflow_forecast` a date series, + `calculated_fields` scalars), so there is nothing to narrow to. +- **`send_email`** — the endpoint serves invoices, quotes, credits, and + purchase orders, and answers with whichever entity was emailed. The `data` + envelope *is* unwrapped, so `T` is the entity itself. + +Within the typed models, a few fields are deliberately left as +`serde_json::Value`: `Statics::gateways`, `Statics::templates`, +`Statics::bulk_updates`, `HealthCheck::extensions`, and +`HealthCheck::queue_data`. These are nested configuration blobs rather than +uniform lookup rows, and the same reasoning applies as for +`Company::settings` — a wrong concrete type is a hard decode error, whereas +`Value` is lossless. ## Compared with the official PHP SDK diff --git a/Cargo.toml b/Cargo.toml index 8f8c808..444aaf5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,9 @@ thiserror = "2" # Already in the tree via reqwest's `url`; used to escape ids before they are # interpolated into request paths. percent-encoding = "2" +# Already in the tree as reqwest's runtime; only `time` is needed, for the +# sleep between retries. +tokio = { version = "1", features = ["time"] } [dev-dependencies] tokio = { version = "1", features = ["rt-multi-thread", "macros"] } diff --git a/README.md b/README.md index 30ffe3b..e82c3f6 100644 --- a/README.md +++ b/README.md @@ -50,15 +50,25 @@ let all_clients = client.list_all_clients(&ListParams::new()).await?; ``` Every request carries a 60-second deadline by default, since `reqwest` -applies none of its own. Override it for slow report exports or large -`list_all_*` walks: +applies none of its own, and transient failures are retried up to 3 times +with exponential backoff. Both are configurable: ```rust use std::time::Duration; +use invoiceninjars::RetryPolicy; -let client = InvoiceNinjaClient::hosted(token)?.with_timeout(Duration::from_secs(300)); +let client = InvoiceNinjaClient::hosted(token)? + .with_timeout(Duration::from_secs(300)) + .with_retry_policy(RetryPolicy::new(5)); ``` +Retrying is deliberately conservative about `POST`. A `POST` that fails with +a 5xx or times out is **not** replayed, because the server may have applied +it before the response was lost and a repeat would create a second invoice +or payment. A 429 is retried for any method, since the server states it +refused the request without processing it. Pass `RetryPolicy::none()` to +disable retrying entirely. + Run the example (requires `IN_API_TOKEN`, optionally `IN_BASE_URL`): ```sh diff --git a/src/client.rs b/src/client.rs index df4e62a..728bc1f 100644 --- a/src/client.rs +++ b/src/client.rs @@ -8,11 +8,12 @@ use crate::error::{ApiErrorBody, Error, Result}; use crate::models::{ BankIntegration, BankTransaction, BankTransactionRule, BulkRequest, Client as ClientModel, ClientGatewayToken, Company, CompanyGateway, CompanyToken, Credit, Design, Document, Expense, - ExpenseCategory, GroupSetting, Invoice, ItemResponse, ListParams, ListResponse, Location, - Payment, PaymentTerm, Product, Project, PurchaseOrder, Quote, RecurringExpense, - RecurringInvoice, RecurringQuote, Subscription, SystemLog, Tag, Task, TaskScheduler, - TaskStatus, TaxRate, User, Vendor, Webhook, + ExpenseCategory, GroupSetting, HealthCheck, Invoice, ItemResponse, ListParams, ListResponse, + Location, Payment, PaymentTerm, Product, Project, PurchaseOrder, Quote, RecurringExpense, + RecurringInvoice, RecurringQuote, SearchResults, Statics, Subscription, SystemLog, Tag, Task, + TaskScheduler, TaskStatus, TaxRate, User, Vendor, Webhook, }; +use crate::retry::{self, RetryPolicy}; /// API root of the hosted Invoice Ninja app. pub const HOSTED_BASE_URL: &str = "https://invoicing.co/api/v1"; @@ -73,6 +74,7 @@ pub struct InvoiceNinjaClient { /// in the derived `Debug`. api_password: Option, timeout: Duration, + retry: RetryPolicy, } impl InvoiceNinjaClient { @@ -121,9 +123,19 @@ impl InvoiceNinjaClient { base_url, api_password: None, timeout: DEFAULT_TIMEOUT, + retry: RetryPolicy::default(), }) } + /// Replaces the retry policy, which defaults to + /// [`RetryPolicy::default`] (3 retries with full-jitter backoff). + /// + /// Pass [`RetryPolicy::none`] to attempt every request exactly once. + pub fn with_retry_policy(mut self, retry: RetryPolicy) -> Self { + self.retry = retry; + self + } + /// Overrides the per-request deadline, which defaults to /// [`DEFAULT_TIMEOUT`]. /// @@ -173,6 +185,63 @@ impl InvoiceNinjaClient { } } + /// Dispatches a built request, retrying transient failures per the + /// configured [`RetryPolicy`]. + /// + /// Every helper goes through here, so retrying is uniform. The request is + /// cloned before each attempt; a body that cannot be cloned (none of ours + /// today, since all payloads are in-memory JSON) simply gets one attempt. + /// + /// Note what is *not* retried: a `POST` that fails with a 5xx or times + /// out is surfaced, because the server may already have applied it and a + /// replay would duplicate an invoice, payment, or email. See + /// [`crate::retry`] for the full rules. + async fn execute(&self, builder: reqwest::RequestBuilder) -> Result { + let mut request = builder.build()?; + let method = request.method().clone(); + let mut attempt = 0u32; + + loop { + let replay = if attempt < self.retry.max_retries { + request.try_clone() + } else { + None + }; + + let delay = match self.http.execute(request).await { + Ok(response) => { + let Some(next) = + replay.filter(|_| retry::status_is_retryable(response.status(), &method)) + else { + return Ok(response); + }; + // A server-supplied Retry-After wins over our backoff, + // but one longer than we are willing to wait is returned + // as the error instead of slept through. + let delay = retry::retry_after(response.headers()) + .unwrap_or_else(|| self.retry.backoff(attempt)); + if delay > self.retry.max_delay { + return Ok(response); + } + request = next; + delay + } + Err(e) => { + let Some(next) = + replay.filter(|_| retry::transport_error_is_retryable(&e, &method)) + else { + return Err(e.into()); + }; + request = next; + self.retry.backoff(attempt) + } + }; + + tokio::time::sleep(delay).await; + attempt += 1; + } + } + /// Builds a client pointed at the hosted app at `https://invoicing.co`. /// /// Equivalent to [`new`](Self::new) with [`HOSTED_BASE_URL`]. Prefer this @@ -218,10 +287,11 @@ impl InvoiceNinjaClient { params: &ListParams, ) -> Result> { let response = self - .req(Method::GET, path) - .query(params) - .query(¶ms.custom) - .send() + .execute( + self.req(Method::GET, path) + .query(params) + .query(¶ms.custom), + ) .await?; let body: ListResponse = Self::decode(response).await?; Ok(body.data) @@ -272,10 +342,11 @@ impl InvoiceNinjaClient { loop { params.page = Some(page); let response = self - .req(Method::GET, path) - .query(¶ms) - .query(¶ms.custom) - .send() + .execute( + self.req(Method::GET, path) + .query(¶ms) + .query(¶ms.custom), + ) .await?; let body: ListResponse = Self::decode(response).await?; let got = body.data.len() as u64; @@ -315,7 +386,7 @@ impl InvoiceNinjaClient { } async fn get_one(&self, path: &str) -> Result { - let response = self.req(Method::GET, path).send().await?; + let response = self.execute(self.req(Method::GET, path)).await?; let body: ItemResponse = Self::decode(response).await?; Ok(body.data) } @@ -325,7 +396,9 @@ impl InvoiceNinjaClient { path: &str, payload: &B, ) -> Result { - let response = self.req(Method::POST, path).json(payload).send().await?; + let response = self + .execute(self.req(Method::POST, path).json(payload)) + .await?; let body: ItemResponse = Self::decode(response).await?; Ok(body.data) } @@ -335,13 +408,15 @@ impl InvoiceNinjaClient { path: &str, payload: &B, ) -> Result { - let response = self.req(Method::PUT, path).json(payload).send().await?; + let response = self + .execute(self.req(Method::PUT, path).json(payload)) + .await?; let body: ItemResponse = Self::decode(response).await?; Ok(body.data) } async fn delete_one(&self, path: &str) -> Result<()> { - let response = self.req(Method::DELETE, path).send().await?; + let response = self.execute(self.req(Method::DELETE, path)).await?; let status = response.status(); let bytes = response.bytes().await?; if status.is_success() { @@ -356,7 +431,9 @@ impl InvoiceNinjaClient { path: &str, payload: &B, ) -> Result> { - let response = self.req(Method::POST, path).json(payload).send().await?; + let response = self + .execute(self.req(Method::POST, path).json(payload)) + .await?; let body: ListResponse = Self::decode(response).await?; Ok(body.data) } @@ -364,7 +441,7 @@ impl InvoiceNinjaClient { /// Returns the raw response body. Used for endpoints that do not answer /// with JSON — PDF streams (`application/pdf`) and CSV report exports. async fn get_raw(&self, path: &str) -> Result> { - let response = self.req(Method::GET, path).send().await?; + let response = self.execute(self.req(Method::GET, path)).await?; let status = response.status(); let bytes = response.bytes().await?; if status.is_success() { @@ -375,7 +452,9 @@ impl InvoiceNinjaClient { } async fn post_raw(&self, path: &str, payload: &B) -> Result> { - let response = self.req(Method::POST, path).json(payload).send().await?; + let response = self + .execute(self.req(Method::POST, path).json(payload)) + .await?; let status = response.status(); let bytes = response.bytes().await?; if status.is_success() { @@ -388,7 +467,7 @@ impl InvoiceNinjaClient { /// Decodes a response that has no envelope and no stable schema /// (`/statics`, `/charts/*`, `/search`). async fn get_unwrapped(&self, path: &str) -> Result { - let response = self.req(Method::GET, path).send().await?; + let response = self.execute(self.req(Method::GET, path)).await?; Self::decode(response).await } @@ -397,7 +476,9 @@ impl InvoiceNinjaClient { path: &str, payload: &B, ) -> Result { - let response = self.req(Method::POST, path).json(payload).send().await?; + let response = self + .execute(self.req(Method::POST, path).json(payload)) + .await?; Self::decode(response).await } @@ -877,8 +958,15 @@ impl InvoiceNinjaClient { /// Posts to a `/charts/{name}` endpoint, e.g. `totals_v2`, /// `chart_summary_v2`, `calculated_fields`, `cashflow_forecast`. /// - /// The chart payloads have no stable published schema, so the response is - /// returned untyped. + /// Deliberately still generic. Unlike the other endpoints here, the + /// eleven charts share no common response shape — `totals_v2` answers + /// with per-currency maps, `cashflow_forecast` with a date series, + /// `calculated_fields` with scalars — so there is nothing to narrow to. + /// Pass `serde_json::Value`, or your own struct for the chart you are + /// calling. + /// + /// `project_analytics` and `project_burnup` take a project id as a + /// further path segment: `chart("project_burnup/", &body)`. pub async fn chart( &self, name: &str, @@ -888,33 +976,58 @@ impl InvoiceNinjaClient { } /// Full-text search across entities. - pub async fn search(&self, body: &B) -> Result { + /// + /// Which buckets of [`SearchResults`] are filled depends on whether the + /// server has Elasticsearch enabled; see the type's docs. + pub async fn search(&self, body: &B) -> Result { + self.search_as(body).await + } + + /// [`search`](Self::search) decoded into a caller-chosen type. + pub async fn search_as(&self, body: &B) -> Result { self.post_unwrapped("search", body).await } /// Fetches the static lookup tables. /// - /// The response is a single object keyed by lookup name; the official PHP - /// SDK exposes `currencies`, `industries`, `languages`, `countries`, - /// `banks`, `payment_types`, and `templates`, and the API also returns - /// `timezones`, `date_formats`, `datetime_formats`, `gateways`, and - /// `sizes`. - /// - /// Each table has its own field set (currencies carry `symbol` and - /// `precision`, countries carry ISO codes), so there's no shared item - /// type; decode into `serde_json::Value`, or into your own struct - /// covering just the tables you need. - /// /// This is a large and near-constant payload — cache it rather than /// fetching per request. - pub async fn statics(&self) -> Result { + pub async fn statics(&self) -> Result { + self.statics_as().await + } + + /// [`statics`](Self::statics) decoded into a caller-chosen type. + /// + /// [`Statics`] types the uniform lookup tables and keeps the rest in + /// `extra`; use this to pin tables it leaves untyped, such as `gateways` + /// or `templates`. + pub async fn statics_as(&self) -> Result { self.get_unwrapped("statics").await } /// Queues an email for an entity. The body selects the target, e.g. /// `{"entity": "invoice", "entity_id": "...", "template": "email_template_invoice"}`. + /// + /// Returns the refreshed entity, so `T` is the type the email was sent + /// for — [`Invoice`], [`Quote`], [`Credit`], or [`PurchaseOrder`]: + /// + /// ```no_run + /// # async fn run(client: &invoiceninjars::InvoiceNinjaClient) -> invoiceninjars::Result<()> { + /// use invoiceninjars::models::Invoice; + /// use serde_json::json; + /// + /// let invoice: Invoice = client + /// .send_email(&json!({ "entity": "invoice", "entity_id": "Wpmbk5ezJn" })) + /// .await?; + /// # Ok(()) + /// # } + /// ``` + /// + /// It stays generic over the entity because the endpoint serves all four, + /// but the `data` envelope is now unwrapped for you — it previously was + /// not, so callers had to model the wrapper themselves. pub async fn send_email(&self, body: &B) -> Result { - self.post_unwrapped("emails", body).await + self.post_one("emails", body).await } /// Refunds a payment. The body takes `id`, optionally `amount`, @@ -931,7 +1044,17 @@ impl InvoiceNinjaClient { } /// Server-side health check (database, queues, and so on). - pub async fn health_check(&self) -> Result { + /// + /// The hosted app does not run the check and answers with a bare + /// message, so most fields of [`HealthCheck`] are `Option` — see its + /// docs. + pub async fn health_check(&self) -> Result { + self.health_check_as().await + } + + /// [`health_check`](Self::health_check) decoded into a caller-chosen + /// type. + pub async fn health_check_as(&self) -> Result { self.get_unwrapped("health_check").await } } diff --git a/src/lib.rs b/src/lib.rs index f21c022..6833e15 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,6 +17,8 @@ mod client; mod error; pub mod models; +mod retry; -pub use client::{InvoiceNinjaClient, HOSTED_BASE_URL}; +pub use client::{InvoiceNinjaClient, DEFAULT_TIMEOUT, HOSTED_BASE_URL}; pub use error::{ApiErrorBody, Error, Result}; +pub use retry::RetryPolicy; diff --git a/src/models/common.rs b/src/models/common.rs index 89ca25a..3cfc0d8 100644 --- a/src/models/common.rs +++ b/src/models/common.rs @@ -1,4 +1,5 @@ use serde::{Deserialize, Deserializer, Serialize}; +use serde_json::Value; #[derive(Debug, Clone, Deserialize)] pub struct ListResponse { @@ -132,6 +133,27 @@ where Ok(Option::::deserialize(deserializer)?.unwrap_or_default()) } +/// Deserializes an id that may arrive as either a JSON string or a number. +/// +/// The `/statics` tables are inconsistent about this: `payment_types` casts +/// its id to a string server-side while the other lookup tables leave theirs +/// as integers. Rather than guess per table — where a wrong choice is a hard +/// decode error that breaks the whole response — both forms are accepted and +/// normalised to `String`. +pub(crate) fn id_as_string<'de, D>(deserializer: D) -> Result +where + D: Deserializer<'de>, +{ + match Option::::deserialize(deserializer)? { + None | Some(Value::Null) => Ok(String::new()), + Some(Value::String(s)) => Ok(s), + Some(Value::Number(n)) => Ok(n.to_string()), + Some(other) => Err(serde::de::Error::custom(format!( + "expected a string or number id, got {other}" + ))), + } +} + /// Body for a bulk action (`POST /{resource}/bulk`). /// /// The action set varies by resource; `archive`, `restore`, and `delete` work diff --git a/src/models/health.rs b/src/models/health.rs new file mode 100644 index 0000000..c6cbee4 --- /dev/null +++ b/src/models/health.rs @@ -0,0 +1,79 @@ +use serde::{Deserialize, Serialize}; +use serde_json::{Map, Value}; + +/// Response of `GET /health_check`. +/// +/// Two different payloads arrive here depending on the deployment. A +/// self-hosted install returns the full `SystemHealth::check()` report +/// modelled below; the hosted app returns only `{"message": "", "errors": +/// []}`, since the check is not meaningful there. Every field defaults, so +/// both decode — test [`system_health`](Self::system_health) rather than +/// assuming the detail fields are populated. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct HealthCheck { + /// Overall verdict. Absent on hosted, where no check runs. + #[serde(default)] + pub system_health: Option, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub api_version: String, + #[serde(default)] + pub php_version: Option, + /// Required PHP extensions and whether each was found. + #[serde(default)] + pub extensions: Value, + #[serde(default)] + pub env_writable: Option, + #[serde(default)] + pub simple_db_check: Option, + #[serde(default)] + pub cache_enabled: Option, + #[serde(default)] + pub exec: Option, + #[serde(default)] + pub open_basedir: Option, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub mail_mailer: String, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub pdf_engine: String, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub queue: String, + /// Queue depth and the most recent failure, when the queue is readable. + #[serde(default)] + pub queue_data: Value, + #[serde(default)] + pub trailing_slash: Option, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub file_permissions: String, + #[serde(default)] + pub exchange_rate_api_not_configured: Option, + #[serde(default)] + pub is_docker: Option, + #[serde(default)] + pub pending_migrations: Option, + + /// Set on the hosted app, which returns a bare message instead of a + /// report. + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub message: String, + + #[serde(flatten)] + pub extra: Map, +} + +/// The PHP version sub-report of a health check. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct PhpVersion { + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub current_php_version: String, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub minimum_php_version: String, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub current_php_cli_version: String, + #[serde(default)] + pub is_okay: Option, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub memory_limit: String, + + #[serde(flatten)] + pub extra: Map, +} diff --git a/src/models/mod.rs b/src/models/mod.rs index 291e84b..d1961db 100644 --- a/src/models/mod.rs +++ b/src/models/mod.rs @@ -12,6 +12,7 @@ mod document; mod expense; mod expense_category; mod group_setting; +mod health; mod invoice; mod location; mod payment; @@ -21,6 +22,8 @@ mod project; mod purchase_order; mod recurring_expense; mod recurring_invoice; +mod search; +mod statics; mod subscription; mod system_log; mod tag; @@ -46,6 +49,7 @@ pub use document::Document; pub use expense::Expense; pub use expense_category::ExpenseCategory; pub use group_setting::GroupSetting; +pub use health::{HealthCheck, PhpVersion}; pub use invoice::{Credit, Invoice, InvoiceItem, Quote}; pub use location::Location; pub use payment::{Payment, PaymentInvoice}; @@ -55,6 +59,8 @@ pub use project::Project; pub use purchase_order::PurchaseOrder; pub use recurring_expense::RecurringExpense; pub use recurring_invoice::{RecurringInvoice, RecurringQuote}; +pub use search::{SearchHit, SearchResults}; +pub use statics::{Currency, DateFormat, StaticItem, Statics}; pub use subscription::Subscription; pub use system_log::SystemLog; pub use tag::Tag; diff --git a/src/models/search.rs b/src/models/search.rs new file mode 100644 index 0000000..3f479cd --- /dev/null +++ b/src/models/search.rs @@ -0,0 +1,65 @@ +use serde::{Deserialize, Serialize}; +use serde_json::{Map, Value}; + +/// Results of `POST /search`. +/// +/// Which buckets are populated depends on how the server is configured: with +/// Elasticsearch enabled the controller fills every entity bucket, and +/// without it only `clients`, `client_contacts`, `invoices`, `projects`, and +/// `settings` are returned. Every field therefore defaults to empty rather +/// than being required, so both shapes decode. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct SearchResults { + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub clients: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub client_contacts: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub invoices: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub quotes: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub credits: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub recurring_invoices: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub purchase_orders: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub expenses: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub vendors: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub vendor_contacts: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub projects: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub tasks: Vec, + /// Matching settings pages rather than records. + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub settings: Vec, + + #[serde(flatten)] + pub extra: Map, +} + +/// One search result. +/// +/// Every bucket uses the same row shape, so this is not entity-specific — +/// use `id` to fetch the full record through the matching `get_*` method. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct SearchHit { + /// Hashed id of the matched record. + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub id: String, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub name: String, + /// Entity kind, as a leading-slash tag like `/client` or `/invoice`. + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub r#type: String, + /// Front-end route for the record, e.g. `/clients/Wpmbk5ezJn`. + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub path: String, + + #[serde(flatten)] + pub extra: Map, +} diff --git a/src/models/statics.rs b/src/models/statics.rs new file mode 100644 index 0000000..e92983e --- /dev/null +++ b/src/models/statics.rs @@ -0,0 +1,112 @@ +use serde::{Deserialize, Serialize}; +use serde_json::{Map, Value}; + +/// The static lookup tables returned by `GET /statics`. +/// +/// This is a large, near-constant payload — cache it rather than fetching it +/// per request. +/// +/// Only the tables with a stable, uniform row shape are typed. `gateways`, +/// `templates`, and `bulk_updates` are left as `Value`: they are nested +/// configuration blobs rather than lookup lists, and the same reasoning +/// applies as for [`Company::settings`](super::Company). Anything not named +/// here — including `einvoice_schema` and the self-host-only `license_key` — +/// is preserved in `extra`. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct Statics { + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub currencies: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub countries: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub industries: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub languages: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub payment_types: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub sizes: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub timezones: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub date_formats: Vec, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub datetime_formats: Vec, + + /// Payment gateway definitions. A nested configuration blob, left + /// untyped. + #[serde(default)] + pub gateways: Value, + /// Email/PDF template defaults. Left untyped for the same reason. + #[serde(default)] + pub templates: Value, + /// Columns that each entity accepts in a bulk update, keyed by entity. + #[serde(default)] + pub bulk_updates: Value, + + #[serde(flatten)] + pub extra: Map, +} + +/// A row in one of the simple lookup tables. +/// +/// `name` arrives already translated into the requesting user's locale. +/// Per-table extras — a country's ISO codes, a language's locale, a +/// timezone's UTC offset — stay in `extra`. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct StaticItem { + /// Normalised to a string: most tables return a number here, but + /// `payment_types` returns a string. + #[serde(default, deserialize_with = "super::common::id_as_string")] + pub id: String, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub name: String, + + #[serde(flatten)] + pub extra: Map, +} + +/// A currency, which carries formatting detail the other lookup tables do +/// not. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct Currency { + #[serde(default, deserialize_with = "super::common::id_as_string")] + pub id: String, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub name: String, + /// ISO 4217 code, e.g. `USD`. + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub code: String, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub symbol: String, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub thousand_separator: String, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub decimal_separator: String, + /// Number of decimal places. `Option` rather than defaulted, since 0 is a + /// meaningful precision (JPY) and must not be confused with "absent". + #[serde(default)] + pub precision: Option, + #[serde(default)] + pub swap_currency_symbol: Option, + #[serde(default)] + pub exchange_rate: Option, + + #[serde(flatten)] + pub extra: Map, +} + +/// A date or datetime format option. +/// +/// `format` is the PHP format string; the `format_moment` and `format_dart` +/// variants for other client libraries stay in `extra`. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct DateFormat { + #[serde(default, deserialize_with = "super::common::id_as_string")] + pub id: String, + #[serde(default, deserialize_with = "super::common::null_to_default")] + pub format: String, + + #[serde(flatten)] + pub extra: Map, +} diff --git a/src/retry.rs b/src/retry.rs new file mode 100644 index 0000000..aff27a9 --- /dev/null +++ b/src/retry.rs @@ -0,0 +1,149 @@ +//! Retry policy for transient failures. +//! +//! The rules here are deliberately conservative about which requests may be +//! replayed. This is a billing API: replaying a `POST` that the server +//! already processed but whose response was lost would create a second +//! invoice or payment, which is far worse than surfacing an error. + +use std::sync::atomic::{AtomicU64, Ordering}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; + +use reqwest::{header, Method, StatusCode}; + +/// How transient failures are retried. +/// +/// The default retries up to 3 times with full-jitter exponential backoff +/// starting at 500ms. Use [`RetryPolicy::none`] to disable retrying. +#[derive(Debug, Clone)] +pub struct RetryPolicy { + /// Retries *after* the initial attempt, so 3 means up to 4 requests. + pub max_retries: u32, + /// Backoff for the first retry; doubles each attempt. + pub base_delay: Duration, + /// Ceiling on any single wait, including one asked for by `Retry-After`. + pub max_delay: Duration, +} + +impl Default for RetryPolicy { + fn default() -> Self { + Self { + max_retries: 3, + base_delay: Duration::from_millis(500), + max_delay: Duration::from_secs(10), + } + } +} + +impl RetryPolicy { + /// Retries `max_retries` times with the default delays. + pub fn new(max_retries: u32) -> Self { + Self { + max_retries, + ..Self::default() + } + } + + /// Disables retrying; every request is attempted exactly once. + pub fn none() -> Self { + Self::new(0) + } + + pub fn base_delay(mut self, base_delay: Duration) -> Self { + self.base_delay = base_delay; + self + } + + pub fn max_delay(mut self, max_delay: Duration) -> Self { + self.max_delay = max_delay; + self + } + + /// Full-jitter exponential backoff: a uniform pick from + /// `[0, min(max_delay, base_delay * 2^attempt)]`. + /// + /// Jittering the whole interval rather than a fraction of it is what + /// stops a set of clients that failed together from retrying together. + pub(crate) fn backoff(&self, attempt: u32) -> Duration { + let exponential = self + .base_delay + .checked_mul(1u32.checked_shl(attempt).unwrap_or(u32::MAX)) + .unwrap_or(self.max_delay) + .min(self.max_delay); + Duration::from_millis(pseudo_random_millis(exponential.as_millis() as u64)) + } +} + +/// Picks a value in `[0, upper]` without pulling in a random-number crate. +/// +/// Decorrelating retries is all this needs to do — it is not used for +/// anything requiring unpredictability. The counter is mixed in so that +/// calls landing in the same nanosecond still diverge. +fn pseudo_random_millis(upper: u64) -> u64 { + if upper == 0 { + return 0; + } + static COUNTER: AtomicU64 = AtomicU64::new(0); + let seq = COUNTER.fetch_add(1, Ordering::Relaxed); + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_nanos() as u64) + .unwrap_or(0); + + // splitmix64 finalizer, enough to spread adjacent seeds. + let mut x = nanos ^ seq.wrapping_mul(0x9E37_79B9_7F4A_7C15); + x ^= x >> 30; + x = x.wrapping_mul(0xBF58_476D_1CE4_E5B9); + x ^= x >> 27; + x = x.wrapping_mul(0x94D0_49BB_1331_11EB); + x ^= x >> 31; + x % (upper + 1) +} + +/// Whether replaying the request can be assumed not to double an effect. +/// +/// `POST` is excluded: it creates records, runs bulk actions, issues refunds +/// and sends email, none of which is safe to repeat blindly. +fn is_idempotent(method: &Method) -> bool { + matches!( + *method, + Method::GET | Method::HEAD | Method::PUT | Method::DELETE | Method::OPTIONS + ) +} + +/// Whether a response status should be retried. +pub(crate) fn status_is_retryable(status: StatusCode, method: &Method) -> bool { + match status.as_u16() { + // Rate limited. The server states it did not process the request, so + // replaying cannot duplicate anything — safe even for POST. + 429 => true, + // The request may or may not have been applied before the error: a + // 502 or 504 can mean the response was lost rather than the work + // undone. Only replay when repeating is harmless. + 500 | 502 | 503 | 504 => is_idempotent(method), + _ => false, + } +} + +/// Whether a transport-level failure should be retried. +/// +/// Same reasoning as a 5xx: a timeout gives no evidence about whether the +/// server processed the request. +pub(crate) fn transport_error_is_retryable(error: &reqwest::Error, method: &Method) -> bool { + (error.is_timeout() || error.is_connect()) && is_idempotent(method) +} + +/// Reads a `Retry-After` delay, which Invoice Ninja sends on throttled +/// responses. +/// +/// Only the delta-seconds form is parsed; the HTTP-date form falls through to +/// normal backoff rather than pulling in a date parser. +pub(crate) fn retry_after(headers: &header::HeaderMap) -> Option { + let seconds: u64 = headers + .get(header::RETRY_AFTER)? + .to_str() + .ok()? + .trim() + .parse() + .ok()?; + Some(Duration::from_secs(seconds)) +} diff --git a/tests/client_requests.rs b/tests/client_requests.rs index 022f2f5..9e78e21 100644 --- a/tests/client_requests.rs +++ b/tests/client_requests.rs @@ -544,3 +544,447 @@ async fn ping_accepts_an_empty_success_body() { client(&server).ping().await.expect("ping should succeed"); } + +// --- retry and backoff -------------------------------------------------- + +use invoiceninjars::RetryPolicy; + +/// Fast policy so the tests do not actually sleep for seconds. +fn quick_retry() -> RetryPolicy { + RetryPolicy::new(3) + .base_delay(Duration::from_millis(1)) + .max_delay(Duration::from_millis(20)) +} + +#[tokio::test] +async fn transient_5xx_on_a_get_is_retried_until_it_succeeds() { + let server = MockServer::start().await; + // First two attempts fail, third succeeds. + Mock::given(method("GET")) + .and(path("/clients")) + .respond_with(ResponseTemplate::new(503)) + .up_to_n_times(2) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/clients")) + .respond_with(ResponseTemplate::new(200).set_body_json(page(1, 1, 1))) + .mount(&server) + .await; + + let got = client(&server) + .with_retry_policy(quick_retry()) + .list_clients(&ListParams::new()) + .await + .expect("a GET should ride out transient 5xx"); + assert_eq!(got.len(), 1); + assert_eq!(server.received_requests().await.unwrap().len(), 3); +} + +/// The safety property that matters most here: a POST may already have +/// created a record even though the response failed, so replaying it could +/// double-charge. It must surface instead. +#[tokio::test] +async fn post_is_not_replayed_on_5xx() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/clients")) + .respond_with(ResponseTemplate::new(500).set_body_json(json!({ "message": "boom" }))) + .mount(&server) + .await; + + let err = client(&server) + .with_retry_policy(quick_retry()) + .create_client(&Default::default()) + .await + .expect_err("a failed create must not be retried"); + assert!(matches!(err, Error::Api { status, .. } if status == 500)); + assert_eq!( + server.received_requests().await.unwrap().len(), + 1, + "exactly one create attempt may reach the server" + ); +} + +/// 429 is the exception: the server states it refused the request, so no +/// work was done and even a POST is safe to repeat. +#[tokio::test] +async fn post_is_retried_on_429_because_nothing_was_processed() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/clients")) + .respond_with(ResponseTemplate::new(429)) + .up_to_n_times(1) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path("/clients")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "data": { "id": "new" } }))) + .mount(&server) + .await; + + let created = client(&server) + .with_retry_policy(quick_retry()) + .create_client(&Default::default()) + .await + .expect("a throttled create should be retried"); + assert_eq!(created.id.as_deref(), Some("new")); + assert_eq!(server.received_requests().await.unwrap().len(), 2); +} + +#[tokio::test] +async fn client_errors_are_never_retried() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/clients/abc")) + .respond_with(ResponseTemplate::new(404).set_body_json(json!({ "message": "not found" }))) + .mount(&server) + .await; + + let err = client(&server) + .with_retry_policy(quick_retry()) + .get_client("abc") + .await + .expect_err("404 is not transient"); + assert!(matches!(err, Error::Api { status, .. } if status == 404)); + assert_eq!(server.received_requests().await.unwrap().len(), 1); +} + +#[tokio::test] +async fn retries_are_bounded_by_the_policy() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/clients")) + .respond_with(ResponseTemplate::new(502)) + .mount(&server) + .await; + + let err = client(&server) + .with_retry_policy(RetryPolicy::new(2).base_delay(Duration::from_millis(1))) + .list_clients(&ListParams::new()) + .await + .expect_err("should give up and surface the last error"); + assert!(matches!(err, Error::Api { status, .. } if status == 502)); + assert_eq!( + server.received_requests().await.unwrap().len(), + 3, + "one initial attempt plus two retries" + ); +} + +#[tokio::test] +async fn retry_policy_none_attempts_once() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/clients")) + .respond_with(ResponseTemplate::new(503)) + .mount(&server) + .await; + + let _ = client(&server) + .with_retry_policy(RetryPolicy::none()) + .list_clients(&ListParams::new()) + .await + .expect_err("retrying is disabled"); + assert_eq!(server.received_requests().await.unwrap().len(), 1); +} + +#[tokio::test] +async fn retry_after_longer_than_max_delay_is_surfaced_rather_than_slept_through() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/clients")) + .respond_with(ResponseTemplate::new(429).insert_header("Retry-After", "3600")) + .mount(&server) + .await; + + let err = tokio::time::timeout( + Duration::from_secs(5), + client(&server) + .with_retry_policy(quick_retry()) + .list_clients(&ListParams::new()), + ) + .await + .expect("must not sleep for the full hour the server asked for") + .expect_err("the throttle should be reported"); + assert!(matches!(err, Error::Api { status, .. } if status == 429)); + assert_eq!(server.received_requests().await.unwrap().len(), 1); +} + +#[tokio::test] +async fn a_short_retry_after_is_honoured() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/clients")) + .respond_with(ResponseTemplate::new(429).insert_header("Retry-After", "0")) + .up_to_n_times(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/clients")) + .respond_with(ResponseTemplate::new(200).set_body_json(page(0, 1, 1))) + .mount(&server) + .await; + + client(&server) + .with_retry_policy(quick_retry()) + .list_clients(&ListParams::new()) + .await + .expect("should wait the requested time and retry"); + assert_eq!(server.received_requests().await.unwrap().len(), 2); +} + +#[tokio::test] +async fn retried_requests_keep_their_body_and_headers() { + let server = MockServer::start().await; + Mock::given(method("PUT")) + .and(path("/clients/abc")) + .and(header("X-Api-Token", "test-token")) + .respond_with(ResponseTemplate::new(503)) + .up_to_n_times(1) + .mount(&server) + .await; + Mock::given(method("PUT")) + .and(path("/clients/abc")) + .and(header("X-Api-Token", "test-token")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "data": { "id": "abc" } }))) + .mount(&server) + .await; + + let body = invoiceninjars::models::Client { + name: "Acme".into(), + ..Default::default() + }; + client(&server) + .with_retry_policy(quick_retry()) + .update_client("abc", &body) + .await + .expect("the replayed PUT should carry the same payload"); + + let sent = server.received_requests().await.unwrap(); + assert_eq!(sent.len(), 2); + for request in &sent { + let json: serde_json::Value = serde_json::from_slice(&request.body).unwrap(); + assert_eq!(json["name"], "Acme", "body must survive the replay"); + } +} + +// --- narrowed response types -------------------------------------------- + +#[tokio::test] +async fn statics_decodes_into_typed_tables() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/statics")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "currencies": [{ + "id": 1, "name": "US Dollar", "code": "USD", "symbol": "$", + "precision": 2, "thousand_separator": ",", "decimal_separator": ".", + "exchange_rate": 1.0, "swap_currency_symbol": false + }], + // payment_types is the table that returns a string id; the rest + // return numbers, and both must land as String. + "payment_types": [{ "id": "1", "name": "Bank Transfer" }], + "industries": [{ "id": 3, "name": "Advertising" }], + "countries": [{ "id": 840, "name": "United States", "iso_3166_2": "US" }], + "date_formats": [{ "id": 1, "format": "d/M/Y", "format_moment": "DD/MMM/YYYY" }], + "gateways": [{ "id": 1, "name": "Stripe", "fields": { "nested": true } }], + "bulk_updates": { "client": ["name"] }, + "license_key": "self-host-only" + }))) + .mount(&server) + .await; + + let statics = client(&server) + .statics() + .await + .expect("statics should decode"); + + let usd = &statics.currencies[0]; + assert_eq!(usd.id, "1", "a numeric id is normalised to a string"); + assert_eq!(usd.code, "USD"); + assert_eq!(usd.precision, Some(2)); + assert_eq!( + statics.payment_types[0].id, "1", + "a string id passes through" + ); + assert_eq!(statics.industries[0].name, "Advertising"); + assert_eq!(statics.date_formats[0].format, "d/M/Y"); + assert_eq!( + statics.countries[0].extra["iso_3166_2"], "US", + "untyped per-table fields stay reachable via extra" + ); + assert!(statics.gateways.is_array(), "gateways stays untyped"); + assert_eq!( + statics.extra["license_key"], "self-host-only", + "unmodeled top-level keys are preserved" + ); +} + +#[tokio::test] +async fn statics_tolerates_tables_it_does_not_model() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/statics")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "currencies": null, + "brand_new_table": [{ "whatever": 1 }] + }))) + .mount(&server) + .await; + + let statics = client(&server) + .statics() + .await + .expect("should not hard-fail"); + assert!(statics.currencies.is_empty(), "null decodes as empty"); + assert!(statics.extra.contains_key("brand_new_table")); +} + +#[tokio::test] +async fn search_decodes_the_non_elasticsearch_shape() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/search")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "clients": [{ + "name": "Acme", "type": "/client", + "id": "Wpmbk5ezJn", "path": "/clients/Wpmbk5ezJn" + }], + "client_contacts": [], + "invoices": [], + "projects": [], + "settings": [{ "name": "Tax Settings", "type": "/settings", "path": "/settings/tax" }] + }))) + .mount(&server) + .await; + + let results = client(&server) + .search(&json!({ "search": "acme" })) + .await + .expect("search should decode"); + + assert_eq!(results.clients[0].name, "Acme"); + assert_eq!(results.clients[0].id, "Wpmbk5ezJn"); + assert_eq!(results.clients[0].r#type, "/client"); + assert_eq!(results.settings[0].name, "Tax Settings"); + // Buckets only the Elasticsearch path returns are absent, not an error. + assert!(results.quotes.is_empty()); + assert!(results.vendors.is_empty()); +} + +#[tokio::test] +async fn search_decodes_the_elasticsearch_shape() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/search")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "clients": [], + "quotes": [{ "name": "Q1", "type": "/quote", "id": "q1", "path": "/quotes/q1" }], + "purchase_orders": [{ "name": "PO", "type": "/purchase_order", "id": "p1", "path": "/x" }] + }))) + .mount(&server) + .await; + + let results = client(&server) + .search(&json!({ "search": "q" })) + .await + .expect("the wider shape should decode too"); + assert_eq!(results.quotes[0].id, "q1"); + assert_eq!(results.purchase_orders[0].name, "PO"); +} + +#[tokio::test] +async fn health_check_decodes_the_self_hosted_report() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/health_check")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "system_health": true, + "api_version": "5.8.56", + "php_version": { + "minimum_php_version": "8.1", + "current_php_version": "8.2.10", + "current_php_cli_version": "8.2.10", + "is_okay": true, + "memory_limit": "512M" + }, + "simple_db_check": true, + "queue": "database", + "pdf_engine": "SnapPDF", + "is_docker": false, + "extensions": [{ "curl": true }] + }))) + .mount(&server) + .await; + + let health = client(&server).health_check().await.expect("should decode"); + assert_eq!(health.system_health, Some(true)); + assert_eq!(health.api_version, "5.8.56"); + assert_eq!(health.queue, "database"); + assert_eq!( + health + .php_version + .expect("php block present") + .current_php_version, + "8.2.10" + ); +} + +/// The hosted app skips the check and returns a bare message, so the same +/// type has to cover both deployments. +#[tokio::test] +async fn health_check_decodes_the_hosted_message_shape() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/health_check")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "message": "", "errors": [] + }))) + .mount(&server) + .await; + + let health = client(&server).health_check().await.expect("should decode"); + assert_eq!( + health.system_health, None, + "no verdict is reported on hosted" + ); + assert!(health.api_version.is_empty()); +} + +/// Regression: the endpoint answers with a `data`-wrapped entity, but this +/// used to be decoded as a bare body, so callers got the envelope. +#[tokio::test] +async fn send_email_unwraps_the_data_envelope() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/emails")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "data": { "id": "inv1", "number": "INV-0001" } + }))) + .mount(&server) + .await; + + let invoice: invoiceninjars::models::Invoice = client(&server) + .send_email(&json!({ "entity": "invoice", "entity_id": "inv1" })) + .await + .expect("should return the refreshed entity"); + assert_eq!(invoice.id.as_deref(), Some("inv1")); + assert_eq!(invoice.number, "INV-0001"); +} + +#[tokio::test] +async fn generic_escape_hatches_still_work() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/statics")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "sizes": [] }))) + .mount(&server) + .await; + + let raw: serde_json::Value = client(&server) + .statics_as() + .await + .expect("statics_as should bypass the typed model"); + assert!(raw["sizes"].is_array()); +}