Add conditional route error response policies - #165
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: 0c1b3f579c
ℹ️ 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".
| responseViewApplicator.apply(shapedResponse); | ||
| } | ||
| selectedPolicy.ifPresent(policy -> applyBodyPolicy(policy, shapedResponse)); | ||
| selectedPolicies.forEach(policy -> applyBodyPolicy(policy, shapedResponse)); |
There was a problem hiding this comment.
When an unconditional error policy uses bodyText(...) or suppressBody() and a matching onErrorWhen(...) policy later selects bodyUsingEntityView(...), applying every body action sequentially leaves the earlier explicit/cleared body in place: applyEntityView only changes the view and neither clears the text override nor restores hasBody. This can occur with a custom authenticator rejection carrying an entity response, and the conditional policy then fails to override the default as the documented layering promises, returning the default text or an empty body instead of the viewed entity.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
The legacy RouteApiResponsePolicyApplier.apply(...) overload currently evaluates header-conditional policies using an implicitly empty header set, which can cause incorrect conditional-policy application when callers don’t supply real request headers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR extends Thingifier’s route-level response shaping so routes can define conditional error response policies (selected via request headers), remove response headers (e.g., WWW-Authenticate), and have these policies applied consistently across both HTTP and direct API execution paths, with corresponding documentation metadata updates.
Changes:
- Add conditional error response policies via
ThingifierApiRouteRule.onErrorWhen(status)with request-header predicates onRouteApiResponsePolicy. - Add response header removal support end-to-end (
RouteApiResponsePolicy.removeHeader→ApiResponse.removeHeader→HttpHeadersBlock.remove). - Update policy application plumbing to pass request headers into
RouteApiResponsePolicyApplierfor HTTP + direct API, plus add tests covering conditional ordering/auth interactions and doc generation.
File summaries
| File | Description |
|---|---|
| thingifier/src/test/java/uk/co/compendiumdev/thingifier/api/response/RouteApiResponsePolicyTest.java | Adds coverage for conditional error policies, header removal, auth rejection shaping, hooks seeing shaped responses, direct API behavior, and doc metadata. |
| thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/ThingifierRestAPIHandler.java | Passes direct-API request headers into response policy application. |
| thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/spec/ThingifierApiRouteRule.java | Introduces storage/accessors for ordered conditional error policies and includes them in generated route metadata. |
| thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/response/RouteApiResponsePolicy.java | Adds header removal + request-header predicate model and matching logic. |
| thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/response/ApiResponse.java | Adds removeHeader to support policy-driven response header removal. |
| thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/http/ThingifierHttpApi.java | Passes HTTP request headers into response policy application so conditional policies can match. |
| thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/http/headers/HttpHeadersBlock.java | Adds case-insensitive header removal. |
| thingifier/src/main/java/uk/co/compendiumdev/thingifier/adapter/http/apihandlers/RouteApiResponsePolicyApplier.java | Applies default + matching conditional policies in order and removes headers as part of policy application. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| public ApiResponse apply( | ||
| final RoutingVerb verb, | ||
| final String publicPath, | ||
| final ApiResponse response, | ||
| final ResponseViewApplicator responseViewApplicator) { | ||
| return apply(verb, publicPath, response, new HttpHeadersBlock(), responseViewApplicator); | ||
| } |
| public boolean matchesRequest(final HttpHeadersBlock requestHeaders) { | ||
| final HttpHeadersBlock headers = | ||
| requestHeaders == null ? new HttpHeadersBlock() : requestHeaders; | ||
| for (RequestCondition condition : requestConditions) { | ||
| if (!condition.matches(headers)) { | ||
| return false; | ||
| } | ||
| } | ||
| return true; | ||
| } |
Summary
Verification
Closes #164