fix(gateway): publish validation + streaming + batch semantics (GW-01, GW-06, GW-08..GW-11, HY-13)
- GW-01: /publish validates every NDJSON chunk against the op's publish_schema (incl. the first-line chunk) via NdjsonChunkStream — terminal Err(INVALID_INPUT)/422 on violation, matching the wire dispatcher's per-chunk contract. Route-level fix; the alkcall spine was explored and rejected (wire validation is pump-side by design). - GW-06: the body is streamed, not buffered — Body::into_data_stream() -> newline-framed BufferedLines -> lazily parsed chunk stream. ADR-068 documents the streamed semantics and the 2 MiB per-line cap. - GW-08: /batch capped at 100 operations (INVALID_INPUT 400). - GW-09: internal-op batch entries now carry generated UUID request ids. - GW-10: first publish line missing `chunk` is rejected INVALID_INPUT. - GW-11: redundant /publish pre-checks removed; enforcement rides on invoke_sink via the shared dispatch spine. - HY-13: the vacuous stub test was replaced by a body-cut-short test. - Adjacent: INVALID_OPERATION_TYPE now maps 422 (with identity) / 401 (without) in error.rs — the route relies on the shared mapper since the pre-checks are gone (GW-03's finding; was a 500 fall-through). Verification: cargo test 211 passed; cargo clippy --all-targets -- -D warnings clean; cargo fmt --check clean.
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
---
|
||||
id: review-001-gateway-publish-semantics
|
||||
name: /publish + /batch gateway fixes (GW-01, GW-06, GW-08..GW-11, HY-13)
|
||||
status: pending
|
||||
status: completed
|
||||
depends_on: []
|
||||
scope: narrow
|
||||
risk: medium
|
||||
@@ -50,12 +50,12 @@ routes:
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] `/publish` test with a `publish_schema`-registered Pub op rejects an invalid chunk (review's gate for this unit)
|
||||
- [ ] First line without `chunk` → `INVALID_INPUT`, not a null publish (test)
|
||||
- [ ] GW-06 decision landed: true streaming or ADR-068 amended; consistent tests + docs
|
||||
- [ ] Batch size capped (test); mixed-shape batch envelopes fixed
|
||||
- [ ] Redundant `/publish` pre-checks removed (dispatch still enforces)
|
||||
- [ ] `cargo test` and `cargo clippy --all-targets -- -D warnings` pass
|
||||
- [x] `/publish` test with a `publish_schema`-registered Pub op rejects an invalid chunk (review's gate for this unit)
|
||||
- [x] First line without `chunk` → `INVALID_INPUT`, not a null publish (test)
|
||||
- [x] GW-06 decision landed: true streaming (implemented — not the ADR amendment); consistent tests + docs
|
||||
- [x] Batch size capped (test); mixed-shape batch envelopes fixed
|
||||
- [x] Redundant `/publish` pre-checks removed (dispatch still enforces)
|
||||
- [x] `cargo test` and `cargo clippy --all-targets -- -D warnings` pass
|
||||
|
||||
## References
|
||||
|
||||
@@ -65,10 +65,80 @@ routes:
|
||||
|
||||
## Notes
|
||||
|
||||
> Agent fills during implementation. If the GW-01 fix goes into
|
||||
> alkcall's `invoke_sink` spine, coordinate the alkcall change (small,
|
||||
> additive) and note it in the summary.
|
||||
- **GW-01 — route-level fix (spine route explored, rejected):** the
|
||||
spine option would put validation inside alkcall's
|
||||
`OperationRegistry::invoke_sink`; but the wire dispatcher's
|
||||
per-chunk validation is intentionally *outside* `invoke_sink` (it
|
||||
lives in `Dispatcher::dispatch`'s `SinkDispatch`/`pump_sink`, which
|
||||
owns the abort channel and init-side error injection). Moving it
|
||||
into `invoke_sink` would have meant a wrapping-stream refactor of
|
||||
that spine plus an alkcall release (alkhttp's dep is the crates.io
|
||||
`0.1.1`). The route now compiles the op's `publish_schema`
|
||||
(jsonschema 0.46, same version alkcall uses) and feeds
|
||||
`NdjsonChunkStream` into `invoke_sink`: every chunk — including the
|
||||
first-line chunk — is validated exactly like the wire dispatcher's
|
||||
`EVENT_PUBLISHED` branch; a violation (or malformed JSON) yields the
|
||||
terminal `Err(INVALID_INPUT)` item the wire's initiator-side
|
||||
`call.error` produces. Both transports now enforce the identical
|
||||
per-chunk contract; no alkcall change was needed.
|
||||
- **GW-06 — true streaming implemented:** `publish_handler` now takes
|
||||
`axum::body::Body` → `into_data_stream()` → `BufferedLines` (newline
|
||||
framer) → `NdjsonChunkStream`; nothing buffers the whole body. The
|
||||
only bound is the per-line cap (2 MiB, mirroring axum's default body
|
||||
limit) plus axum's own whole-body default on transport reads. ADR-068
|
||||
gained a "Body handling (streamed, not buffered)" section + a
|
||||
consequence line instead of an amendment — the original step-4
|
||||
wording was already "stream each NDJSON line"; the docs now state the
|
||||
implemented mechanics.
|
||||
- **GW-08:** `MAX_BATCH_OPERATIONS = 100`; over-cap → 400
|
||||
`INVALID_INPUT` (test covers over-cap rejection and at-cap dispatch).
|
||||
- **GW-09:** `not_found_envelope_json` now generates a
|
||||
`uuid::Uuid::new_v4()` request id, so every entry in one `/batch`
|
||||
response body carries the same envelope shape.
|
||||
- **GW-11:** all four `/publish` pre-checks (internal-op probe,
|
||||
existence probe, ACL probe, op-type probe) removed; visibility/ACL/
|
||||
handler-kind/type enforcement rides on `invoke_sink` via the shared
|
||||
dispatch spine, exactly like `/call`//`/batch`.
|
||||
- **HY-13:** the vacuous stub was replaced by
|
||||
`publish_client_disconnect_before_dispatch_signals_error_item`
|
||||
(body cut short mid-stream → the handler's `PublishStream` observes
|
||||
the `Err` item) plus the malformed-later-line test now asserts the
|
||||
real 422 `INVALID_INPUT` mapping.
|
||||
- **Adjacent (in-scope necessity):** `error.rs` gained the
|
||||
`INVALID_OPERATION_TYPE` mapping (422 with identity / 401 without) —
|
||||
the dropped pre-checks mean `/publish` now relies on the shared
|
||||
mapper for the non-Pub-op case, and the old fall-through mapped it to
|
||||
500 (GW-03's finding; the route previously special-cased it to 400).
|
||||
Coordinates with review-001-gateway-stream-errors (GW-03) — that task
|
||||
should verify/doc the mapping rather than re-implement it.
|
||||
|
||||
## Summary
|
||||
|
||||
> Filled on completion.
|
||||
Implemented all seven findings in `src/gateway/routes.rs` (+ the
|
||||
`error.rs` mapping addition + an ADR-068 doc note):
|
||||
|
||||
- **GW-01:** `/publish` validates every NDJSON chunk against the op's
|
||||
compiled `publish_schema` (incl. the first-line chunk) before it
|
||||
reaches the sink — a terminal `Err(INVALID_INPUT)`/422 on violation,
|
||||
message + details matching the wire dispatcher. No alkcall spine
|
||||
change (rejected: the wire validation is pump-side by design; a
|
||||
route-level fix achieves both-transport enforcement without touching
|
||||
alkcall's spine or taking a new release dependency).
|
||||
- **GW-06:** true streaming — `Body::into_data_stream()` →
|
||||
newline-framed `BufferedLines` → lazily parsed, schema-validated
|
||||
`NdjsonChunkStream` → `invoke_sink`. ADR-068 documents the streamed
|
||||
semantics + 2 MiB per-line cap.
|
||||
- **GW-08:** `/batch` capped at 100 operations (`INVALID_INPUT` 400).
|
||||
- **GW-09:** internal-op batch entries get generated UUID request ids
|
||||
(single envelope shape per response).
|
||||
- **GW-10:** first publish line missing `chunk` → `INVALID_INPUT` 400.
|
||||
- **GW-11:** `/publish` pre-checks removed; enforcement via `invoke_sink`.
|
||||
- **HY-13:** vacuous test deleted; replaced by an early-terminate body
|
||||
test (client-cut body → handler sees the `Err` item, 422).
|
||||
|
||||
Tests: 24 gateway-route tests added/updated (schema reject/accept,
|
||||
first-chunk validation, no-schema passthrough, first-line-missing-chunk,
|
||||
oversized-line cap, cap+disconnect semantics, batch cap, batch
|
||||
request-id shape, `INVALID_OPERATION_TYPE` 401/422 matrix + `error.rs`
|
||||
mapping tests). `cargo test` 199 passed; `cargo clippy --all-targets --
|
||||
-D warnings` clean; `cargo fmt --check` clean.
|
||||
Reference in New Issue
Block a user