The read tier now also fetches each row raw and fails if a typed row lost a
key, which is what catches a field the models drop.
The write tier creates a purchase order, location, webhook and recurring
invoice from bare `Default` models, since those are exactly what broke
against a real server, and exercises the uploads and the client extras. The
password tier merges two throwaway clients and purges the survivor.
Writes stay behind the existing guard: `IN_SMOKE_WRITE_HOST` must equal the
host in `IN_BASE_URL`, so a write only ever reaches a server that was named
explicitly. It prints PASS/FAIL per check and never the contents of a
record. `smoke.yml` runs it on demand only (`workflow_dispatch`), with the
write tier gated on the `write_host` input naming the instance.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Twice now a mock written from the upstream source passed while a real server
disagreed — the system-log timestamps and the `/subscriptions/steps` shape —
and `frequency_id` and several empty-string fields were wrong the same way.
Reading the source harder was not going to find the next one.
`tests/fixtures/*.json` hold the *shapes* of real responses: every key, with
its JSON type, and no values at all. `real_shapes.rs` decodes each one and
checks that a decode plus re-serialise loses no key, so a field the models
silently drop now fails a test. It also refuses a fixture holding any real
value, and refuses a fixture that no test reads.
`seed_sample_data.rs` fills a disposable instance with one row of each kind
and `capture_shapes.rs` captures the shapes back out with GET requests only,
so the set can be refreshed when a divergence turns up.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Invoice Ninja returns `null` for many nominally-string fields and omits
others, so one model that fails on either takes the whole response down with
it. Rather than trusting each field to carry the right serde attribute, this
serialises `Default`, then nulls and separately removes each field in turn,
and requires the result to still decode.
A wrong *type* is deliberately not covered: that is meant to be a hard
error, so it is not something to absorb.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Forty-one hand-written endpoints, taking the crate from 284 methods to 325:
the per-resource extras (webhook retry, task sorting, default design,
subscription steps, the company gateway and bank integration actions, and
the client extras), the password-protected destructive actions (client and
company purges, client and vendor merges, user invite and purge), the
activity list, feed, per-entity feed and note, email history, and the six
preview routes.
The password-protected actions send `X-API-PASSWORD`, falling back to
`X-API-PASSWORD-BASE64` when the password holds a control character, which
cannot go in a header value. None of the destructive actions are replayed
on a 5xx, and neither are the notes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every one of these was wrong in a way the mock tests could not see, because
the mocks were written from the same upstream source the models were.
- `SystemLog.created_at`/`updated_at` are integer timestamps, not the date
strings the OpenAPI spec declares.
- `frequency_id` arrives as a numeric string on recurring invoices and
expenses, so both now decode through `int_or_numeric_string`.
- `Location.country_id`, `PurchaseOrder.status_id` and `Webhook.format` are
omitted when empty. The server runs `ConvertEmptyStringsToNull`, so an
unset `String` sent as `""` becomes NULL and then fails a non-null column
or an `integer` rule — which meant a create from a `Default` model could
not succeed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
These are the types the per-resource extras and the activity endpoints
need, split out so they land before the code and tests that use them.
`MessageResponse` covers the `{"message": "..."}` body that the action
endpoints answer with, defaulting `message` so a reply carrying none still
decodes. `int_or_numeric_string` goes in alongside them: the spec calls
`frequency_id` a number, but a real 5.13.50 server sends the string `"5"`,
and a strict `i64` turned that into a hard decode error *after* the record
had already been created.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The suite runs against mock servers written from the upstream source, so
it cannot catch a place where a real server disagrees with that reading.
`examples/smoke_test.rs` can: it lists every resource, checks the
pagination walk against a single oversized request, probes the per_page
cap, and exercises statics, search and health_check. The upload wire
format (POST plus `_method=PUT`) was read from the framework source
rather than observed, so its write tier is the real verification of it.
It prints PASS/FAIL per check and never the contents of a record.
Writes are guarded. `IN_SMOKE_WRITE_HOST` must equal the host in
`IN_BASE_URL`, so a write can only reach a server that was named
explicitly. `IN_BASE_URL` has no default, unlike the other example,
because defaulting to the hosted app is the wrong failure for a tool
that can write. The write tier leaves a gap in the client number
sequence and a soft-deleted row even after cleanup, so it is documented
as disposable-instance only.
Running it against a dead port exposed a diagnosability gap:
`Error::Http` printed "error sending request for url (...)" and nothing
else. reqwest's Display stops at the outermost layer, and the reason
("Connection refused", a DNS failure, a TLS error) lives in `source()`.
The error now renders its whole cause chain.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
First item on the GOALS.md roadmap. `upload_{x}(id, files, is_public)`
attaches files through `/{resource}/{id}/upload` and returns the
refreshed record. Verified against upstream that all 16 controllers
answer with `itemResponse($x->fresh())`.
Three details that look wrong but are not, each checked in the upstream
source rather than taken from the published spec:
- The request is a POST carrying `_method=PUT`. The route is registered
PUT, but PHP does not parse a multipart body on a real PUT, so the
files never arrive. Laravel's kernel calls
`enableHttpMethodParameterOverride()`, turning the spoofed method back
into a PUT before routing. The spec documents POST with `_method=POST`,
which would 405 against these routes.
- `is_public` is sent as 1/0. Laravel's `boolean` rule accepts only
true, false, 0, 1, '0' and '1' under strict comparison, so "true"
fails validation.
- Uploads are never retried. An upload adds a document, so it is not
idempotent whatever its verb, and a replay after a lost response would
attach the file twice. This falls out of reqwest's multipart body being
un-cloneable, which nothing in the types enforces, so a test pins it
for 500, 503 and 429.
An upload with no files, a nameless file or an unparseable MIME type is
refused locally (`Error::InvalidUpload`), since the server answers an
empty upload with a success that changed nothing.
Corrects two earlier claims:
- GOALS.md and COVERAGE.md said uploads would make `create_document`
possible. They do not: `DocumentController::store` is an empty stub,
and an upload is the only way a document is created. There is no
`create_document`.
- The documented method count (265) had been stale since the typed
`statics`/`search`/`health_check` commit, which added three `_as`
methods without updating it. Recounted from the source: 284 = 210 +
18 + 12 + 16 uploads + 28 hand-written.
Also fixes a README example, `let statics: Value = client.statics()`,
that stopped compiling when `statics` lost its type parameter.
`edocument/upload` is left out: it imports an expense from an
e-document rather than attaching a file, so it moves to the
import/export roadmap item.
Enabling `multipart` adds mime, mime_guess and unicase; MSRV stays 1.88,
re-verified by running the suite on it. 101 tests, up from 91.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`Cargo.toml` named github.com/bryant/invoiceninjars, a guess from the
git author name that was never a real repository. It now points at the
mirror that actually exists, so the crates.io "Repository" link will
resolve.
Gitea is the source of truth and a push mirror copies every commit to
GitHub. Anything written to the GitHub copy is overwritten by the next
sync, so a merged pull request there would silently vanish. README and
GOALS.md say so, and direct issues and pull requests to Gitea.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"No ORM, session, or caching layer" bundled several decisions with very
different costs, and read as refusing things that are in fact fine.
Split into the three that genuinely conflict, each with its reason:
lazy loading (models would have to hold a client handle, breaking
`Default`-constructibility and the `extra` round trip), dirty tracking
(cannot diff a flattened map of unmodelled fields), and transparent
entity caching.
The caching reason is the one worth recording: `BaseController`'s shared
response path sends no `ETag`, `If-None-Match`, or `Cache-Control`, so a
cached entity cannot be cheaply revalidated. On an API that handles money
that is a correctness hazard rather than a missed optimisation. Writing
the evidence down so the question is not reopened without it.
Two things the old wording excluded by accident are now open questions
instead: typed `include` helpers, since eager loading is a stringly
parameter where a typo is a silent no-op; and an opt-in cache for
`/statics`, which is near-constant and not money. Also parks the
`updated_at` delta-sync parameter as the shape a local mirror would
need, noted as a different product rather than a plan.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
The advisory job was marked `continue-on-error`, which GitHub renders as
a green check with an annotation — a signal nobody reads. Removed, so a
failure shows as a real red X.
What now keeps it from blocking merges is branch protection: it has to
stay out of the required status checks. That is a repo setting rather
than anything in this file, so it is called out in ci.yml and CLAUDE.md.
Expect the job red occasionally through no fault of the change under
test; that is the cost of it being visible.
Also runs weekly (Mondays 06:00 UTC). A Rust release lands on its own
schedule, so on push/PR alone breakage could sit unnoticed for weeks.
The pinned `test` and `msrv` jobs skip scheduled runs — a pinned
toolchain against a locked dependency graph cannot change on a timer.
Adds a top-level `permissions: contents: read`, since the workflow needs
nothing more.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pinning CI meant a future Rust release could break the build without
anyone noticing until the pin moved. This job tracks latest stable and
runs the test suite and clippy, so that breakage is visible early.
Marked `continue-on-error`, so it reports rather than gates: a new
clippy lint landing in stable should not fail a pull request that did
not cause it. The pinned `test` job remains the gate.
Note that GitHub renders a `continue-on-error` job as passing with an
annotation rather than as failed. If it should be more visible, make it
a required check and drop the flag.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The dev shell tracked `stable.latest` and CI used `@stable`, so nothing
pinned a version and nothing declared a floor. A consumer on an older
toolchain hit a confusing error deep in a dependency rather than cargo
saying the version was too old, and a new stable release could turn a
lint into an unrelated red build.
- `rust-version = "1.88"` in Cargo.toml. The floor comes from the
dependency graph, not this crate's code: reqwest pulls in url -> idna
-> idna_adapter -> icu_*, and the icu crates require 1.88. Verified by
running the full suite against 1.88, not inferred.
- flake.nix pins the dev toolchain to 1.99.1 and adds an `msrv` devShell,
so the floor can be rechecked with
`nix develop .#msrv --command cargo test`.
- CI pins the main job to the same 1.99.1 and adds a job building on
1.88. Lint and format stay on the pinned toolchain only, since their
output varies by release.
Note the tradeoff: with CI pinned, a future stable that breaks the build
will not be noticed until the pin moves. Worth adding a non-blocking
`stable` job if that matters.
CLAUDE.md records the three places that must agree when either version
moves, and how to recheck the floor after a dependency bump.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Second hardening pass, over the code added in 23c660d.
Error reporting:
- Decode failures now carry an excerpt of the body that arrived. The
serde message alone ("expected value at line 1 column 1") gives no hint
that the response was an HTML error page from a proxy or a login
redirect from a wrong base URL. The excerpt is collapsed to one line
and truncated so a large body cannot become a large error.
- API errors with an empty or message-less body report the status reason
("Not Found") rather than rendering blank.
- `Error` is `#[non_exhaustive]`, so future variants are not breaking.
Retry bounds:
- `RetryPolicy::max_elapsed` (120s default) bounds the time one call may
spend across attempts. The request timeout is per attempt, so the
previous defaults could spend four minutes on a call that kept timing
out — a surprise for anyone reading `with_timeout(60)` as a bound on
the call.
- `backoff` uses saturating arithmetic; a large `max_delay` could
overflow the jitter span and panic in a debug build.
- Response bodies are drained before a retry so the connection returns to
the pool instead of the next attempt opening a fresh one.
Credentials:
- Test pinning that neither the token nor the password appears in `Debug`
output. `reqwest`'s `Client` prints its default headers, so the
sensitive flag set when building them is the only thing keeping the
token out of logs, and nothing enforced that.
Also drops `Option::is_none_or`, which raised the minimum toolchain to
1.82 for no benefit over a match.
91 tests, up from 80.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Expands coverage to every `Route::resource` group in the v5 API plus
`users` (265 methods), and fixes several defects in the shared request
path that the new breadth made more exposed.
Coverage:
- 25 new models, each keeping the `extra` flatten map and the
`null_to_default` convention so unmodeled fields round-trip.
- `crud_methods!` split out of `resource_methods!` for the three groups
with no `/bulk` route, where a generated `bulk_*` would only 405.
- COVERAGE.md added as the detailed inventory, verified against
`routes/api.php` on v5-stable.
Fixes, each confirmed against upstream source or an observed request:
- `list_all_*` silently truncated. `BaseController` caps page size at
`min(abs(per_page), 5000)`, so the "short page means last page" rule
stopped early and returned a partial list as if complete. Termination
now prefers `meta.pagination.total_pages` (decoded but previously
discarded), and `per_page` is clamped so the fallback stays sound.
- `per_page(0)` paged forever: the check `got < 0` has no solution. It
now falls back to the default size, with a MAX_PAGES backstop that
raises `Error::PaginationLimit` rather than quietly truncating.
- Ids were interpolated into paths raw, so a `/` or `?` in an id
retargeted the request. All sites now use `seg(id)?`.
Escaping alone is insufficient: the WHATWG rules `url` implements
decode `%2E` to `.` before resolving dot segments, so an escaped `..`
still collapses the path. Empty and dot-only ids are rejected.
- No request timeout. reqwest applies none, so a stalled connection hung
the caller indefinitely. Adds a 60s default and `with_timeout`.
- Base URL is validated at construction; a missing scheme used to 404
on every call instead of failing once.
Adds tests/client_requests.rs: 26 tests over the request plumbing
against a wiremock loopback server, including regressions for the
truncation and non-termination bugs. No network or credentials needed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Covers Clients, Invoices, Quotes, Credits, Payments, Products, Expenses,
Projects, Tasks, and Vendors, each with list, list_all, get, create,
update, and delete, generated by the resource_methods! macro.
Every model carries a #[serde(flatten)] extra map so fields the API
returns but the crate does not model survive a get/update round-trip
instead of being silently dropped.
Includes a Nix dev shell (Rust via rust-overlay), a GitHub Actions CI
workflow running build/test/fmt/clippy, and round-trip tests for the
extra-preservation invariant.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
awkawbandClaude Opus 5