Add fixed-route single instance response shaping - #163
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c922764e0e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!rule.hasFixedIdentifierMapping()) { | ||
| return ApiResponse.error( | ||
| 500, |
There was a problem hiding this comment.
When respondWithSingleInstance() is placed on a normal generated instance rule—the added ThingifierApiSpecTest does this for /api/todos/{id}—a successful request returns 500 solely because the rule has no fixed mapping, even when response.hasReturnedInstance() is already true. This turns an otherwise valid generated GET into a server error; accept an already-satisfied singleton response before enforcing fixed-route conversion, or reject this configuration during specification setup.
Useful? React with 👍 / 👎.
| case COLLECTION: | ||
| return collectionResponse(rule, publicPath, response); |
There was a problem hiding this comment.
When a fixed route uses ResponseShape.COLLECTION, this branch makes the runtime response a collection, but OpenAPI generation never consumes RoutingDefinition.responseShape(): ThingifierApiSpec.applyFixedRoutePayloads() still registers the singular entity payload for successful fixed routes, and Swaggerizer derives its schema from that payload name. Generated clients therefore expect an object while the server returns the collection wrapper, so the documented payload/schema also needs to honor this shape.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
Response shaping currently turns non-fixed routes with a shape override into 500 errors, which makes the new API unsafe to use outside fixed-identifier routes.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces an explicit, route-level “response shape” contract (ResponseShape) so fixed-identifier routes can opt into returning a single entity instance (and be documented as such), with shaping applied before route response policies/hooks.
Changes:
- Added
ResponseShapeand route-level configuration onThingifierApiRouteRule(responseShape(...),respondWithSingleInstance()) and copied this metadata into generatedRoutingDefinitions. - Implemented response shaping in
RouteApiResponsePolicyApplierso successful fixed-route responses can be coerced into single-instance (or collection) form before success policies and hooks run. - Added tests covering route metadata propagation, fixed-route runtime behavior (legacy override, hooks, HEAD header policies), and OpenAPI schema expectations.
File summaries
| File | Description |
|---|---|
| thingifier/src/test/java/uk/co/compendiumdev/thingifier/api/spec/ThingifierApiSpecTest.java | Adds tests that response-shape overrides are recorded on route rules and copied into generated route definitions. |
| thingifier/src/test/java/uk/co/compendiumdev/thingifier/api/spec/ThingifierApiFixedRouteTest.java | Adds fixed-route runtime and OpenAPI tests for single-instance shaping, legacy behavior preservation, hooks, HEAD ordering, and ENSURE_EXISTS behavior. |
| thingifier/src/test/java/uk/co/compendiumdev/thingifier/api/response/RouteApiResponsePolicyTest.java | Adds tests for multiple-instance rejection under SINGLE_INSTANCE and explicit COLLECTION wrapping behavior. |
| thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/spec/ThingifierApiRouteRule.java | Adds route-level response shape configuration, getters, and propagation into routing definitions. |
| thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/spec/ResponseShape.java | Introduces the ResponseShape enum (DEFAULT, SINGLE_INSTANCE, COLLECTION). |
| thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/docgen/RoutingDefinition.java | Stores response-shape metadata for documentation generation. |
| thingifier/src/main/java/uk/co/compendiumdev/thingifier/adapter/http/apihandlers/RouteApiResponsePolicyApplier.java | Applies response-shape transformation before response policies and view/body handling. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (!rule.hasResponseShapeOverride() | ||
| || response.isErrorResponse() | ||
| || response.hasABodyOverride() | ||
| || response.getStatusCode() < 200 | ||
| || response.getStatusCode() >= 300) { | ||
| return response; | ||
| } |
Summary
ResponseShapeandrespondWithSingleInstance()for fixed-resource routes.Verification
mvn -pl thingifier verifythingifier-1.5.6-SNAPSHOTto local Maven for API ChallengesCloses #162