add rust axum server example, bugfixes

[dgunzy]

Signed-off-by: Daniel Guns danbguns@gmail.com

Follow-up to #91, which left the rust validation package with nothing to plug into: firestone generated a rust client CLI but no rust server, so the rules landed in a hand-written handler rather than a generated one.

What

Two generators, one document. openapi-generator -g rust-axum turns the OpenAPI document firestone already produces into the server — the router, the typed models, per-operation authentication and request validation. That is not firestone's to duplicate. What it leaves behind is a trait with one required method per operation, 52 of them here, and nothing compiles until they all exist.

firestone generate server --language rust writes those: every method, wired to a Backend trait, enforcing the resources' validation rules on the way past. Both halves derive from the same document, so they cannot disagree about an operation id, a model name or a response variant — unit tests pin the three naming rules (CreatePostal_code → CreatePostalCode, "Response for OK" → Status200_ResponseForOK).

examples/addressbook/server-rs/
├── api/   openapi-generator: router, models, auth, request validation
└── app/   firestone: the implementation of the traits it declares

Firestone regenerates handlers.rs and tests/api.rs every time; everything else is scaffolded once and then yours, so the Backend you implement and the tokens you accept survive a regeneration. --force overrides.

The auth split is the part worth knowing. Which operations need a token is in the schema and the generated server enforces it; what counts as a valid token is a deployment question and lives in the auth.rs firestone writes. No middleware of firestone's own: router() hands back a plain axum::Router, so rate limiting, CORS and request ids are tower-http layers in main.rs and a deployment decision rather than something a schema can state.

On the size

Most of the diff is api/, openapi-generator's output. It is committed on purpose so the example builds and runs with cargo run, no generator or JVM needed, the same way the python example commits its models/ and apis/. The half that is firestone's is app/; the templates, module and tests come to about 2k lines, and that is the reviewable surface.

Bugfixes

Generating a server that actually serves turned up several, most only reachable once something tried to answer a request:

• Every HEAD operation was undeployable. firestone emitted {"default": ...} as the only response. default is a fallback shape, not a status, so a server generator has nothing to put on the response; rust-axum resolves it to response.status(0), which is not a status, and the response fails to build. All 11 HEAD operations answered 500.
• A spec could reference a schema it never defined. With methods.resource absent, the paths were generated for every collection method while the components were generated for none, leaving a dangling $ref to CreateThing. Any server generator failed on it.
• The rust CLI never sent its bearer token. It set config.api_key, but our schemas declare scheme: bearer and the generated client reads bearer_access_token for that. --api-key was accepted and silently dropped, so a correct token still got a 401.
• Python CLI boolean options were malformed. The / separating a click flag pair was missing, so --is-valid became --is-valid--no-is-valid and every create or update carrying a boolean failed with a TypeError.
• The committed python CLI did not parse — SyntaxError: duplicate argument 'address_key', stale output from an older generator.
• python -m firestone was a no-op — if __name__ == "main":, missing dunders.

Also guarded the example rule in addressbook.yaml, which read self.is_valid unguarded and so 500'd once an attribute endpoint handed it a resource that never had one.

Verification

226 python tests, black / pylint / pycodestyle clean.

make verify-rust regenerates both halves into a throwaway workspace, runs cargo clippy -D warnings and cargo test over them, and does the same for the committed crates plus a cargo fmt --check. It also builds six different operation sets through both generators, because the addressbook happens to use every code path and a two-line schema does not: that leg is what catches an import the handlers do not use, or an auth.rs generated for a schema that secures nothing. CI installs openapi-generator, and the script now fails rather than skips if it is missing there, so that leg cannot quietly stop running.

Ran rather than only compiled: 401 without a token, 201 with one, 201 for bearer in lowercase, 422 on person_must_exist, 409 on person_is_not_in_use, 200 on HEAD, 501 on an operation firestone will not guess at, 404 through the ErrorHandler seam.

Known gaps

• Firestone writes a handler body only where the mapping is unambiguous. An operation with no request body, a collection DELETE or PATCH, and a POST or PATCH on an attribute all answer 501: a POST to an embedded collection appends and a PATCH merges, and neither is a replace.
• firestone never emits a requestBody for PATCH, which is a spec bug of the same family as the HEAD one. Left for its own change, since it moves every PATCH signature.
• Query parameters are parsed and validated by the generated server but not applied; filtering belongs in your Backend.
• The rust CLI prints raw Debug on an error, so the problem document is there but escaped.

[ebourgeois]

Should fix before merge

1. The HEAD bugfix has no test, contrary to the description. The PR says each bugfix has a test that fails against the old template, but the only HEAD test is the pre-existing test_openapi.py::test_head, which just does assertIsNotNone(responses) and passes both before and after. Add an assertion that 200 is a key and "default" isn't. It's a one-liner and it's the fix most likely to regress silently.

2. A created resource never gets its key back. InMemory::create mints "{resource}-{n}" and the handler returns from_json(created) where created is the submitted body, so the response Addressbook has address_key: None and the client has no way to GET, PUT or DELETE what it just made. Either the handler should inject the key into the response under the resource's key name (the generator knows it, it's op.key), or create should return a body that already carries it. This is a correctness gap in the demo flow, not just a rough edge.

3. pascal() is defined twice, once in spec/_base.py (as a Jinja filter) and again in spec/server_rust.py, byte-identical. Import the one from _base and keep a single definition.

4. _failures() is dead code. It's computed for every operation but never referenced in the template or anywhere else. Either drop it or wire it in (the 4xx variants would let the handler return typed responses for rule failures instead of always going through ErrorHandler, which might have been the intent).

Worth doing, not blocking

• The Rust side of the server crate has no tests of its own. The one #[tokio::test] is the generated validation-examples test; auth.rs, backend.rs (including merge and resolve_path) and error.rs have none, and the "401 without a token, 201 with one, 422, 409, 200 on HEAD, 404" checks you list were done by hand. An axum::Router plus tower::ServiceExt::oneshot makes those cheap to automate, and it would be the only thing catching a template change that compiles but misbehaves. Also note verify.sh runs cargo clippy on the committed server crate but never cargo test, so even the one existing test doesn't run in CI.
• CI doesn't install openapi-generator, so the "regenerate both halves into a throwaway workspace" branch of verify.sh is skipped on every run; only the committed crates get checked. Worth either adding a Java + openapi-generator step, or saying so in the PR so nobody assumes the regeneration path is covered.
• _success() is called twice per operation (once for success, once inside implemented). Trivial, but bind it once.
• constant_time_eq early-returns on length mismatch, which leaks token length. Fine for an example, but the comment above accepted promises timing safety, so either drop the claim or compare against a fixed-length hash.
• verify.sh line 14 still says make verify-validations-rust; the target was renamed.
• _body_type falls back to serde_json::Value for unknown scalar types, but the generated api crate won't have that as a param type, so it would fail to compile rather than degrade. Probably fine to leave since firestone controls the schema, but a comment would help.

Nits

• Cargo.toml.jinja2: the comment # LazyLock, and Option::is_none_or in the validation engine. reads like it lost its first half.
• .gitignore for server-rs/target/? Didn't see one added; worth confirming it's covered by an existing pattern.

[dgunzy]

Should fix before merge

1. **The HEAD bugfix has no test, contrary to the description.** The PR says each bugfix has a test that fails against the old template, but the only HEAD test is the pre-existing `test_openapi.py::test_head`, which just does `assertIsNotNone(responses)` and passes both before and after. Add an assertion that `200` is a key and `"default"` isn't. It's a one-liner and it's the fix most likely to regress silently.

2. **A created resource never gets its key back.** `InMemory::create` mints `"{resource}-{n}"` and the handler returns `from_json(created)` where `created` is the submitted body, so the response `Addressbook` has `address_key: None` and the client has no way to GET, PUT or DELETE what it just made. Either the handler should inject the key into the response under the resource's key name (the generator knows it, it's `op.key`), or `create` should return a body that already carries it. This is a correctness gap in the demo flow, not just a rough edge.

3. **`pascal()` is defined twice**, once in `spec/_base.py` (as a Jinja filter) and again in `spec/server_rust.py`, byte-identical. Import the one from `_base` and keep a single definition.

4. **`_failures()` is dead code.** It's computed for every operation but never referenced in the template or anywhere else. Either drop it or wire it in (the 4xx variants would let the handler return typed responses for rule failures instead of always going through `ErrorHandler`, which might have been the intent).

Worth doing, not blocking

* The Rust side of the server crate has no tests of its own. The one `#[tokio::test]` is the generated validation-examples test; `auth.rs`, `backend.rs` (including `merge` and `resolve_path`) and `error.rs` have none, and the "401 without a token, 201 with one, 422, 409, 200 on HEAD, 404" checks you list were done by hand. An `axum::Router` plus `tower::ServiceExt::oneshot` makes those cheap to automate, and it would be the only thing catching a template change that compiles but misbehaves. Also note `verify.sh` runs `cargo clippy` on the committed server crate but never `cargo test`, so even the one existing test doesn't run in CI.

* CI doesn't install openapi-generator, so the "regenerate both halves into a throwaway workspace" branch of `verify.sh` is skipped on every run; only the committed crates get checked. Worth either adding a Java + openapi-generator step, or saying so in the PR so nobody assumes the regeneration path is covered.

* `_success()` is called twice per operation (once for `success`, once inside `implemented`). Trivial, but bind it once.

* `constant_time_eq` early-returns on length mismatch, which leaks token length. Fine for an example, but the comment above `accepted` promises timing safety, so either drop the claim or compare against a fixed-length hash.

* `verify.sh` line 14 still says `make verify-validations-rust`; the target was renamed.

* `_body_type` falls back to `serde_json::Value` for unknown scalar types, but the generated api crate won't have that as a param type, so it would fail to compile rather than degrade. Probably fine to leave since firestone controls the schema, but a comment would help.

Nits

* `Cargo.toml.jinja2`: the comment `# LazyLock, and Option::is_none_or in the validation engine.` reads like it lost its first half.

* `.gitignore` for `server-rs/target/`? Didn't see one added; worth confirming it's covered by an existing pattern.

Sorry - should have it in Draft - still working on this. Will take this into account before the next push

[dgunzy]

Ready for another review @ebourgeois

[ebourgeois]

Re-review of this branch — 9 findings, most severe first.

Blocking (generated crate does not compile / generated test fails):

1. needs_models misses path/query param types → handlers.rs missing models import
2. Created resource's key is dropped by the serde round-trip → generated *_create_returns_a_usable_key test panics

Also worth fixing before merge:
3. Unconditional use serde_json::json; trips the repo's own -D warnings gate
4. Scaffold-protected lib.rs desyncs when security/validations are enabled on a later regeneration
5. only_admins_may_invalidate rejects partial updates that don't touch is_valid

Lower impact: 6–9 (hard-coded test seed, format-blind int mapping, unescaped Cargo/main metadata, docs contract).

Worth noting that 1–3 all sit in blind spots of test/rust/verify.sh: its shape matrix either defaults methods.resource to all methods (so a POST body always exists) or declares no instance path, and it runs cargo clippy but never cargo test. Widening that matrix would catch all three.

[dgunzy]

Thanks — all nine are fixed, plus both nits. Pushed.

1, 2, 3 all reproduced in the widened matrix before I touched anything, which was the useful part of your review: verify.sh now builds 11 shapes, every one declaring methods.resource explicitly so the all-methods default cannot manufacture a POST body, and it runs cargo test rather than only clippy. Your five named shapes are in there.

2 I fixed by not claiming the key rather than by injecting harder — serde genuinely cannot carry a field the struct lacks, so key_field is now gated on the key being a declared property of the request model. No claim and no test for a schema that does not declare one; the addressbook keeps all three because it does.

4 you are right that scaffolding lib.rs is unsound, since it declares modules that depend on the document. Moved to GENERATED.

5 confirmed: a non-admin PUT that omits is_valid got a 403. Added || !has(self.is_valid) and an example pinning it, which fails without the new term.

6 seed dropped. 7 rust_type() now honours int32/int64/float/double. 9 the docs now carry a table of what is rewritten (handlers.rs, lib.rs, resolver.rs, validation/, tests/api.rs) against what is scaffolded.

8 was the interesting one. Writing the test found the same bug a level up: a newline or colon in --description produced an unparseable OpenAPI document, because openapi.jinja2 interpolated it raw. Fixed there and in asyncapi.jinja2 with | tojson, which is the three-line diff to the committed spec.

Nits: the Cargo comment lost its first half and reads properly now; server-rs/target/ is covered by the existing target/ pattern, confirmed with git check-ignore.

234 python tests, the gate green across all 11 shapes.

[ebourgeois]

Please resolve conversations.

[dgunzy]

Done! Comments to address were in one above, so did not resolve with a comment.