From fbde9a9bb7b73fda731d592ec22aa3dbd634f17f Mon Sep 17 00:00:00 2001 From: Andrew Bryant Date: Fri, 23 Oct 2026 19:25:59 -0400 Subject: [PATCH] Write down the project's goals, non-goals, and roadmap Several decisions in here look like oversights until you know the reasoning: the curated model subset with everything else in `extra`, the still-generic `chart`, not retrying `POST`, the deliberate divergences from the official PHP SDK. That reasoning was spread across code comments and commit messages, where it is easy to miss and easy to "fix". GOALS.md states: - What the crate is for: complete coverage, and not quietly losing data, truncating results, or succeeding when it did not. - Seven goals, each tied to a decision visible in the code, so the rationale survives the next refactor. - Non-goals, including no sync API, no caching or ORM layer, no modelling of every field, and no retrying of non-idempotent requests. - Publication posture: on crates.io, pre-1.0, no stability promise, pin an exact version. MSRV may rise with dependencies. - A roadmap ordering the COVERAGE.md gaps by value over effort, starting with multipart uploads, which is the only thing blocking `create_document`. - Open questions, left open rather than papered over: whether auth routes belong here at all given they are a different auth model, whether per-chart types earn their keep, streaming for large exports, and the untested `Error::PaginationLimit`. README, COVERAGE.md, and CLAUDE.md link to it. No changelog entry: this adds no behaviour, per the rule in CLAUDE.md. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 8 +++ COVERAGE.md | 4 ++ GOALS.md | 161 ++++++++++++++++++++++++++++++++++++++++++++++++++++ README.md | 4 ++ 4 files changed, 177 insertions(+) create mode 100644 GOALS.md diff --git a/CLAUDE.md b/CLAUDE.md index c4b1b07..9e81625 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,6 +6,14 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co An async Rust client for the [Invoice Ninja](https://www.invoiceninja.com/) v5 API. +**`GOALS.md` states what the project is for, what it deliberately is not, +and what is still undecided.** Read it before making a design call — several +choices in here look like oversights until you know the reasoning (the +curated model subset, the untyped `chart`, not retrying `POST`, the +divergences from the official PHP SDK). If a change would contradict a +stated goal or settle an open question, update `GOALS.md` in the same +change. + ## Development environment This project uses Nix for toolchain management (Rust via `rust-overlay`): diff --git a/COVERAGE.md b/COVERAGE.md index 4f25f1c..19fe74a 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -2,6 +2,10 @@ What this crate wraps of the Invoice Ninja v5 API, and what it doesn't. +Full coverage is a stated goal, so the gaps below are a to-do list rather +than a permanent boundary — [`GOALS.md`](GOALS.md) puts them in rough +priority order. + Checked against `routes/api.php` on the `v5-stable` branch of the [invoiceninja repo](https://github.com/invoiceninja/invoiceninja), which declares 34 `Route::resource` groups and 274 explicit route declarations diff --git a/GOALS.md b/GOALS.md new file mode 100644 index 0000000..2275d1f --- /dev/null +++ b/GOALS.md @@ -0,0 +1,161 @@ +# Goals + +What this crate is trying to be, what it is deliberately not, and what is +still open. The point of writing this down is that several decisions here +look like oversights until you know the reasoning — this is where the +reasoning lives. + +## What this is for + +A complete, honest async Rust client for the Invoice Ninja v5 API. + +*Complete* in that every endpoint the API exposes should eventually be +reachable through it. *Honest* in that it should not quietly lose data, +quietly truncate results, or quietly succeed when it did not. + +## Goals + +### 1. Never lose data in a round trip + +A `get` followed by an `update` must not drop fields the crate does not know +about. Every model carries `#[serde(flatten)] extra`, and the modelled +fields are a curated subset rather than an attempt at the whole schema. + +This is why `custom_value1..4`, `custom_surcharge1..4`, and fields with +ambiguous upstream types are intentionally unmodelled — `extra` carries them +losslessly, whereas a wrong concrete type is a hard decode error that breaks +the entire response. + +### 2. Tolerate upstream inconsistency, fail on real problems + +Invoice Ninja returns `null` for many nominally-string fields, and the same +field name can differ in type across resources (`frequency_id` is a number on +recurring invoices and a string on subscriptions; `/statics` returns +`payment_types.id` as a string and every other table's as a number). + +Decoding should absorb that — hence `null_to_default` and `id_as_string` — +without becoming so permissive that genuine errors pass silently. + +### 3. Prefer a loud failure to a quiet wrong answer + +The bug worth remembering: `list_all_*` used to stop when a page came back +shorter than requested, which the server's `per_page` cap made wrong, so it +returned a partial list indistinguishable from a complete one. Silent +truncation is the worst failure mode this crate can have. + +Concretely: pagination terminates on the paginator's own `total_pages`, a +runaway walk raises `Error::PaginationLimit` rather than returning what it +has, and an id that cannot safely be a path segment is rejected instead of +being mangled into a request against a different endpoint. + +### 4. Be safe by default where the API handles money + +`POST` is never replayed on a 5xx or a timeout, because the server may have +applied it before the response was lost and a retry would create a second +invoice, payment, or email. A 429 is retried for any method, because the +server states it refused the request without processing it. + +When a safety default and a convenience conflict here, safety wins, and the +reasoning goes in a comment so it is not later optimised away. + +### 5. Make failures diagnosable + +An error should say enough to act on. Decode failures carry an excerpt of +the body that actually arrived, because "expected value at line 1 column 1" +does not reveal that a proxy returned an HTML error page. API errors keep +the whole per-field `errors` map, not just the first message. + +### 6. Verify against the source, not against assumptions + +Field names and types come from Invoice Ninja's `openapi/api-docs.yaml`; the +set of endpoints comes from `routes/api.php` and the controllers, because +the published spec is incomplete for routes. Claims in `COVERAGE.md` are +checked against the upstream repository rather than inferred. + +### 7. Stay testable without credentials + +The whole suite runs offline: model decoding against fixtures, request +plumbing against a `wiremock` server on loopback. Nobody should need an +Invoice Ninja account to contribute. + +## Non-goals + +- **Not a port of the official PHP SDK.** It is a useful cross-check, but + thinner, and two of its behaviours are bugs not to copy. See the + comparison in `COVERAGE.md`. The divergences — JSON instead of + `form_params`, keeping the whole validation `errors` map, exposing bulk + `download` only on PDF-rendering entities — are deliberate. +- **No sync/blocking API.** Async only. +- **No ORM, session, or caching layer.** The crate wraps endpoints. It does + not model relationships between entities or cache responses, beyond + advising that `/statics` is worth caching. +- **No modelling of every field.** See goal 1; breadth of endpoints is a + goal, breadth of typed fields is not. +- **No retrying of non-idempotent requests.** See goal 4. +- **No API stability before 1.0.** See below. + +## Publication and compatibility + +Published on crates.io so it is installable, with **no stability promise +before 1.0**. Pin an exact version; expect breaking changes in minor +releases. `CHANGELOG.md` records them, and breaking changes should be +legible there rather than discovered at compile time. + +`Error` is `#[non_exhaustive]` so that adding variants is not itself a +breaking change. + +**MSRV is 1.88**, set by the dependency graph rather than by this crate's +code. It may rise when dependencies require it; that is a changelog entry, +not a blocker. It should never rise because a convenience API was used +without checking — see the note in `CLAUDE.md`. + +## Roadmap: closing the coverage gaps + +All 34 `Route::resource` groups are wrapped. Full coverage is the target, so +`COVERAGE.md`'s gap list is a to-do list, not a permanent boundary. Roughly +in order of value over effort: + +1. **Multipart uploads** — 16 `PUT /{resource}/{id}/upload` routes plus + `edocument/upload`. Blocked only on enabling reqwest's `multipart` + feature. This is also what makes `create_document` possible, so it + unblocks the one obviously missing piece of an otherwise complete + resource. +2. **Per-resource extras** — mechanical, and they reuse existing patterns: + `webhooks/{id}/retry`, `tasks/sort`, `designs/set/default`, + `client_gateway_tokens/{id}/setAsDefault`, `subscriptions/steps`, the + client extras (`updateTaxData`, `show_settings`, `check_vat`, …), the + bank-integration and company-gateway extras. +3. **Password-protected actions** — `with_api_password` already makes these + reachable; only the methods are missing. Client purge and merge, vendor + merge, company purge, user invite and purge. +4. **Preview, activities, and email history** — distinct response shapes, so + each needs its own hand-written method and probably its own model. +5. **Public invitation-key downloads** — eight routes that take no auth + token, so they do not fit the authenticated request path and need a + different entry point. +6. **Import/export and the integrations** — Stripe, QuickBooks, einvoice + (17 routes), Yodlee, Nordigen, calendar. The largest block and the least + SDK-shaped; worth doing last and possibly in pieces. + +Keep `COVERAGE.md` current as these land — it names exact counts, so it goes +stale quietly. + +## Open questions + +These are genuinely undecided. Deciding one is a change to this file. + +- **Auth routes** (`/login`, `/logout`, `/refresh`, passkeys) are in the gap + list, but they are a different authentication model from the API token + this client is built around. Including them may mean a second client type + rather than more methods on this one. Not obviously in scope. +- **`chart` is still generic** over `T: DeserializeOwned`, because the + eleven chart endpoints share no response shape. Per-chart structs would + be eleven small models of unstable payloads; unclear whether that earns + its keep. +- **Large responses are buffered entirely into memory.** PDF downloads and + CSV report exports go through `Vec`. Streaming would be better for + big exports but changes those signatures. +- **`Error::PaginationLimit` has no test**, because reaching it needs + 10,001 mock round trips. Making `MAX_PAGES` injectable purely for a test + may be worse than the gap. +- **When to go 1.0**, and what the API needs to settle first. diff --git a/README.md b/README.md index b23d63d..1a24ec7 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,10 @@ An async Rust client for the [Invoice Ninja](https://www.invoiceninja.com/) v5 A Requires Rust 1.88 or newer. +> **Pre-1.0: no API stability promise.** Pin an exact version and expect +> breaking changes in minor releases; `CHANGELOG.md` records them. +> [`GOALS.md`](GOALS.md) covers what the crate is for and what it is not. + ## Development Enter the dev shell (Rust toolchain via `rust-overlay`):