Skip to content

feat: serve rest-json from the generic CRUD engine - #139

Merged
skyoo2003 merged 2 commits into
mainfrom
feat/rest-json-engine
Sep 5, 2026
Merged

skyoo2003 merged 2 commits into
mainfrom
feat/rest-json-engine

Conversation

@skyoo2003

Copy link
Copy Markdown
Owner

Summary

The CRUD engine could only read the X-Amz-Target JSON protocols, so 59 registered rest-json services were routed but served nothing. The operation was always recoverable — every rest-json operation is bound to a method and URI template in the model, and the generated per-service routers have matched on that pair since day one. The gateway just threw the answer away.

This PR adds no services. It changes what the engine can read, and 28 already-registered services stop being registered-only as a result.

Related Issue

Refs .claude/prds/aws-service-coverage-100.prd.md — answers Open Question 1 ("Does the claim include non-JSON-protocol services?"). Yes for rest-json; query and rest-xml stay outside and are Milestone 5.

Fixes #

Changes

internal/shared/httproute (new) — the URI matcher lifted out of router.go.tmpl, where it was emitted into 148 generated packages and could not be imported. PathParams and OperationRoute became type aliases, so cloudfront, efs and their tests compile and pass unchanged. Net −16,728 lines across 136 regenerated routers.

internal/shared/crud — OpMeta carries the REST binding; Handle takes a Call and resolves the operation through httproute; parameters are read from query, body, then path labels, with the path label authoritative because the URI is what addresses the resource.

internal/codegen — isJSONProtocol becomes engineServable and admits rest-json; the CRUD registry carries each operation's method and URI.

internal/gateway — body buffering moves from crud.JSONProtocol to crud.Servable. rest-xml stays unbuffered: S3 bodies are large binary uploads and the engine refuses the protocol anyway.

Fidelity manifest — now publishes EngineWired per service. TestFidelityManifestCoversCRUDRegistry assumed CRUD-registry membership implies the engine serves an operation; that broke honestly on apigatewayv2 and xray, whose hand-written providers never return ErrUnhandledOp and so are never routed to the engine. The manifest was right and the test was inferring — it now reads the fact.

Fleet effect, before a single new service is added

Before After
Services serving ≥1 operation 117 145
Registered-only 31 3
auto-crud operations 1,415 2,200
unimplemented operations 3,119 2,334

The 3 that remain are not a protocol problem: forecastquery and the two SageMaker Runtime variants have no CRUD-shaped operation at all.

Test Plan

  • CGO_ENABLED=0 go test ./... — all pass
  • make test-compat — 790 passed, 2 skipped
  • rm -rf internal/generated && make codegen — no drift (matches the CI codegen-drift job)

RED evidence (a standalone RED commit is impossible here: the repo's pre-commit go vet refuses a non-compiling tree, so it is recorded in the commit bodies):

  • engine — go test ./internal/shared/crud/: undefined: Call; unknown field Method in struct literal of type OpMeta; FAIL [build failed]
  • codegen — go test ./internal/codegen/ -run TestServiceCRUDData: crudOpData has no field or method Method/URI; FAIL [build failed]
  • gateway — TestServiceRouter_RESTJSONBodyReachesEngine: "request body did not reach the engine": expected "g1", actual <nil>. The other two rest-json gateway cases already passed, which is what showed routing was live and only buffering was missing.

End-to-end: polly, qbusiness and codeguru-reviewer failed test_registered_only_service_declines_cleanly with "returned 200 but nothing serves this service" — the harness correctly detecting that they now serve. They moved to ENGINE_SERVED_SERVICES in the same commit.

New tests worth reading: TestEngineRESTJSONUnmatchedPathIsUnclassified and TestServiceRouter_RESTJSONUnmatchedPathDeclinesCleanly — a path the route table does not know must decline with InvalidAction, never a fabricated success. That guarantee is what the whole coverage claim rests on.

Checklist

  • Self-reviewed the code
  • Added/updated tests
  • Lint/format passes (golangci-lint run)
  • Updated documentation (if applicable) — docs/coverage.md
  • Added a Changie changelog fragment for user-facing changes

The matcher lived only inside router.go.tmpl, so it was emitted into 148
generated packages and could not be imported. The CRUD engine needs the
same answer from the same data to resolve a rest-json operation, and
writing it a 149th time is how the two would drift.

PathParams and OperationRoute become type aliases, so the four
hand-written callers (cloudfront, efs and their tests) compile and pass
unchanged — that is the behavioural proof this is a refactor.

RED: go test ./internal/shared/httproute/ failed to build —
  match_test.go:12:11: undefined: Params
  match_test.go:85:15: undefined: MatchURI
  match_test.go:163:14: undefined: Route
  FAIL ... [build failed]
(a standalone RED commit is impossible here: the repo's pre-commit go-vet
hook refuses a non-compiling tree, so the evidence is recorded here.)

GREEN: go test ./internal/shared/httproute/ -v — 20/20 subtests PASS.
Full suite: CGO_ENABLED=0 go test ./... all pass.

gen_router_test.go swaps its "type OperationRoute struct" assertion for
one on the delegation, which is what stops the algorithm being re-inlined.

Net -16,728 lines across 136 regenerated routers.
rest-json carries no X-Amz-Target header, so the engine had nothing to
classify and 59 registered services served nothing. The operation was
always recoverable: every rest-json operation is bound to a method and URI
template in the model, and the generated per-service routers have matched
on that pair since day one. The gateway just threw the answer away.

crud.OpMeta now carries the REST binding, crud.Handle takes a Call and
resolves the operation through internal/shared/httproute, and the gateway
buffers rest-json bodies so a Create sees its parameters. rest-xml stays
unbuffered: S3 bodies are large binary uploads and the engine refuses the
protocol anyway.

Fleet effect, before a single new service is added:
  services serving >=1 op   117 -> 145
  registered-only            31 -> 3
  auto-crud operations    1,415 -> 2,200
  unimplemented           3,119 -> 2,334

The 3 that remain are not a protocol problem: forecastquery and the two
SageMaker Runtime variants have no CRUD-shaped operation at all.

RED (engine): go test ./internal/shared/crud/ — undefined: Call;
  unknown field Method/URI in struct literal of type OpMeta. FAIL [build].
RED (codegen): go test ./internal/codegen/ -run TestServiceCRUDData —
  crudOpData has no field or method Method/URI. FAIL [build].
RED (gateway): TestServiceRouter_RESTJSONBodyReachesEngine —
  "request body did not reach the engine": expected "g1", actual <nil>.
  The other two rest-json gateway cases already passed, which is what
  showed routing was live and only buffering was missing.
GREEN: go test ./... all pass; make test-compat 790 passed, 2 skipped.

Also: the fidelity manifest now publishes EngineWired per service.
TestFidelityManifestCoversCRUDRegistry assumed registry membership implied
the engine serves an operation; that broke honestly on apigatewayv2 and
xray, whose hand-written providers never return ErrUnhandledOp and so are
never routed to the engine. The manifest was right and the test was
inferring — it now reads the fact instead.

polly, qbusiness and codeguru-reviewer move to ENGINE_SERVED_SERVICES;
personalize-runtime leaves REGISTERED_ONLY_SERVICES (GetRecommendations
classifies as a Get) but has no parameterless read to smoke-test with, so
the manifest is its record.
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests Test code and test infrastructure codegen Smithy codegen and generated code labels Sep 5, 2026
@skyoo2003
skyoo2003 merged commit c04a3bc into main Sep 5, 2026
8 checks passed
@skyoo2003
skyoo2003 deleted the feat/rest-json-engine branch September 5, 2026 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codegen Smithy codegen and generated code documentation Improvements or additions to documentation tests Test code and test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant