feat(server): add an OpenAPI document for the HTTP API - #4212
justinmclean wants to merge 2 commits into
Conversation
Describe all 46 HTTP operations in core/server/openapi.json, with a test that fails if the document and the route table drift apart.
|
Thanks for the PR. It is labeled Slash commands (own line, regular comment) move it around the queue:
See CONTRIBUTING.md for details. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4212 +/- ##
=============================================
- Coverage 87.48% 66.89% -20.60%
- Complexity 1575 1576 +1
=============================================
Files 1280 1277 -3
Lines 223192 180498 -42694
Branches 186555 143863 -42692
=============================================
- Hits 195267 120744 -74523
- Misses 23229 55143 +31914
+ Partials 4696 4611 -85
🚀 New features to boost your workflow:
|
LudwigJMarx
left a comment
LudwigJMarx
left a comment
There was a problem hiding this comment.
Read the test and the document against d4630c7fe. I could not run cargo test:
no Rust toolchain on this machine. Everything below is from reading the source
plus checks I ran myself, so treat the test's own behaviour as unverified by me.
Checked independently
| Check | Result |
|---|---|
Routes parsed out of src/http.rs against paths |
45 operations, identical sets, every operationId equals the registered handler |
.route( outside src/http.rs |
only the three /ui routes in src/http/web.rs, excluded as the module docs say |
$ref targets |
640, all resolve |
operationId uniqueness |
no duplicates |
| Path template parameters | all declared through components/parameters, all required: true, none declared that the template does not contain |
Unused components/schemas and components/parameters |
none |
| Tags used but not declared | none |
Operations without responses |
none |
| The 17 request body schemas against the Rust request types | match |
The body comparison is where I expected to find something and did not.
StoreConsumerOffset flattens Consumer and renames id to consumer_id, and
the document has consumer_id. SendMessages has a hand-written Deserialize
that takes only partitioning and messages, and the document says exactly
that, down to user_headers as oneOf array, base64 string or null, and id
as oneOf integer or string. minItems: 1 on messages matches
IggyMessagesBatch::validate, which send_messages calls.
Scope, not a defect
The guard compares (path, method, operationId). Request bodies, parameters and
responses sit outside it. Nothing fails if UpdateUser gains a field, or a field
becomes required, and the document keeps the old shape.
This matters for #4214, which points a contributor at this document instead of
the Rust source. My change there compares ApiSchema.ts against openapi.json
on route, method, body fields, required fields and nullability. The route half is
guarded by your test. The body half rests on the document being right today,
which I checked by hand above, and on it staying right, which nothing checks. Is
guarding the bodies something you want later, or deliberately out of scope?
One nit
is_cfg_test matches contains("test") against the whole attribute token
string, so #[cfg(not(test))] and #[cfg(feature = "latest")] are skipped as
well. Nothing in src/http.rs hits it today, and the failure mode is a puzzling
"operation with no route" rather than a silent pass. Reading attr.meta as a
path list would be exact.
|
Thanks for checking the document by hand, especially the request bodies, but it would be good if you could run cargo test. Guarding the bodies is out of scope for this PR. Deriving schemas from the Rust types would add a new server dependency, and it would still miss hand-written deserialisers like SendMessages. Good catch on is_cfg_test. I'll change it to skip only #[cfg(test)]. |
is_cfg_test matched any cfg whose text contained "test", so cfg(not(test)) and feature names like "latest" were skipped too. Parse the predicate as a bare ident instead.
|
@LudwigJMarx pardon for my question, but are you AI agent? |
3 participants
Which issue does this PR address?
N/A
Rationale
The HTTP API currently has no complete description anywhere. The docs don't list its endpoints, even though the FAQ calls HTTP the most accessible transport. The only other sources are the server.http examples, which have a mistake, and the Web UI's hand-written types, which are out of date. So anyone calling the API with curl, from a language without an SDK, or through an AI tool has nothing reliable to work from. The document fixes that, and its test stops the list of routes from drifting again. It's useful rather than urgent: most SDK users go through the binary TCP protocol and never touch HTTP.
What changed?
Adds an OpenAPI 3.1 document for the server's HTTP API in core/server/openapi.json, covering all 46 operations with their parameters, request and response bodies, authentication, status codes and the Iggy-View and Iggy-Durability headers. The document was written from the handler code. A new test, core/server/tests/openapi_routes.rs, parses the route table in src/http.rs and fails if a route is missing from the document, the document has an operation with no route, or an operationId does not match its handler. It does not check request or response fields, so a renamed field can still drift; generating the document from annotations would close that gap later.
Local Execution
AI Usage
AI used to generate, reviewed and tested by a human.