From 14bda45e5cad797237ad454d87ae01864ff1a895 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Sat, 23 May 2026 00:29:46 +0200 Subject: [PATCH 01/11] docs: Spec for decorator ScopedValue scope fix --- ...-23-decorator-scoped-value-scope-design.md | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-23-decorator-scoped-value-scope-design.md diff --git a/docs/superpowers/specs/2026-05-23-decorator-scoped-value-scope-design.md b/docs/superpowers/specs/2026-05-23-decorator-scoped-value-scope-design.md new file mode 100644 index 0000000..989b778 --- /dev/null +++ b/docs/superpowers/specs/2026-05-23-decorator-scoped-value-scope-design.md @@ -0,0 +1,101 @@ +# Decorator runs inside interceptor ScopedValue scope + +## Problem + +`README.md` ("Combining the two") promises that a `ResponseDecorator` +sees `ScopedValue` bindings established by a `RequestInterceptor`: + +> Decorators run inside the interceptor's `ScopedValue` binding (the +> decorator transforms the `Response` returned by `next.proceed()`, +> which is still on the call stack), so `CORRELATION_ID.get()` / +> `TENANT_ID.get()` see the bound values. + +The implementation contradicts this contract. +`DispatchHandler.handle` runs the entire interceptor chain to +completion, then loops decorators after the chain has unwound: + +```java +Response response = invoke(0, request, handler); // pops all + // ScopedValue + // frames +for (ResponseDecorator decorator : decorators) { + response = decorator.decorate(request, response); // no bindings +} +``` + +A decorator that calls `CORRELATION_ID.get()` throws +`NoSuchElementException`. + +## Fix + +Move the decorator loop into the base case of `invoke()`, so +decorators run after `handler.handle(request)` and before the call +unwinds through the interceptor frames. + +`DispatchHandler.handle`: + +```java +public void handle(HttpExchange exchange) { + Request request = CURRENT.get(); + RequestHandler handler = handlers.get(request.operationId()); + Response response = invoke(0, request, handler); + exchange.setAttribute(RESPONSE_ATTR, response); + renderer.render(exchange, response); +} + +private Response invoke(int idx, Request req, RequestHandler h) { + if (idx == interceptors.size()) { + Response response = h.handle(req); + for (ResponseDecorator d : decorators) { + response = d.decorate(req, response); + } + return response; + } + return interceptors.get(idx) + .around(req, () -> invoke(idx + 1, req, h)); +} +``` + +## Consequences + +- Decorators see all interceptor `ScopedValue` bindings. README + contract honored. +- Interceptors observe the decorated `Response` on the way back up. + An interceptor wrapping `next.proceed()` in `try`/log can record + the final status, headers, and body produced by the handler + + decorators. README already implies this. +- If a decorator throws, the exception propagates through the + interceptor chain. Previously it skipped interceptors and went + straight to `ExceptionFilter`. Interceptors that wrap + `next.proceed()` in `try`/`catch` now observe decorator failures. + Worth a release note. + +## Out of scope + +`AfterResponseHook` runs from `RequestPreparationFilter` after the +interceptor chain has unwound and therefore does not see interceptor +`ScopedValue` bindings. That is a separate gap with a different +fix (the hook would need to fire from inside the interceptor frame, +which changes the documented "interceptor wraps the handler" +contract). Users who need per-request context in an after-callback +can register a closure via `request.afterResponse(Runnable)` from +inside the interceptor — the runnable captures the resolved values. + +## Tests + +Add to `DispatchHandlerTest` (or create one): + +1. **Decorator sees interceptor-bound `ScopedValue`.** Register an interceptor that binds a `ScopedValue` and a decorator that reads it and stamps a header. Assert the header is present on the rendered response. Without the fix this throws `NoSuchElementException`. +2. **Interceptor observes decorated response.** Register an interceptor that captures the `Response` returned by `next.proceed()`. Register a decorator that adds a header. Assert the captured response carries the decorator-added header. +3. **Decorator failure is visible to interceptors.** Register an interceptor with `try`/`catch` around `next.proceed()` and a decorator that throws. Assert the interceptor's catch block ran. + +## Files touched + +- `src/main/java/com/retailsvc/http/internal/DispatchHandler.java` +- `src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java` + (extend existing or add) + +## README + +No change. The current wording in "Combining the two" is correct; +the implementation is being brought into compliance. From 9c28e01c499eafb9a606ebd87b37f2e849797cb2 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Sat, 23 May 2026 00:32:32 +0200 Subject: [PATCH 02/11] docs: Plan for decorator ScopedValue scope fix --- ...2026-05-23-decorator-scoped-value-scope.md | 375 ++++++++++++++++++ 1 file changed, 375 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-23-decorator-scoped-value-scope.md diff --git a/docs/superpowers/plans/2026-05-23-decorator-scoped-value-scope.md b/docs/superpowers/plans/2026-05-23-decorator-scoped-value-scope.md new file mode 100644 index 0000000..09be68d --- /dev/null +++ b/docs/superpowers/plans/2026-05-23-decorator-scoped-value-scope.md @@ -0,0 +1,375 @@ +# Decorator ScopedValue Scope Fix — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make `ResponseDecorator` instances run inside the `ScopedValue` bindings established by `RequestInterceptor`s, matching the contract documented in `README.md` ("Combining the two"). + +**Architecture:** Move the decorator loop from `DispatchHandler.handle` into the base case of the recursive `invoke(...)` helper. Decorators then execute after `handler.handle(request)` but *before* control unwinds through the interceptor stack, so every interceptor's `ScopedValue.where(...).call(...)` frame is still live when a decorator runs. A side effect is that interceptors observe the *decorated* Response on the way back up — explicitly endorsed by the README. + +**Tech Stack:** Java 25, JDK `com.sun.net.httpserver.HttpServer`, JUnit 5, AssertJ, Mockito, Maven. + +**Spec:** `docs/superpowers/specs/2026-05-23-decorator-scoped-value-scope-design.md` + +--- + +## File Structure + +- Modify: `src/main/java/com/retailsvc/http/internal/DispatchHandler.java` — relocate decorator loop. +- Modify: `src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java` — three new unit tests pinning the new contract. + +No README change. No public API change. No new files. + +--- + +## Task 1: Failing test — decorator sees interceptor-bound ScopedValue + +**Files:** +- Modify: `src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java` + +- [ ] **Step 1: Add imports and a class-level test ScopedValue** + +Open `src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java`. Add the following imports near the existing ones (keep alphabetical order with the rest of the file): + +```java +import com.retailsvc.http.RequestInterceptor; +import com.retailsvc.http.ResponseDecorator; +import java.util.concurrent.atomic.AtomicReference; +``` + +Add a static field inside the class body, immediately after the class declaration line: + +```java + private static final ScopedValue CORRELATION_ID = ScopedValue.newInstance(); +``` + +- [ ] **Step 2: Add a helper that builds a DispatchHandler with interceptors and decorators** + +Add this overload of `dispatcher` to the class (place it directly under the existing `dispatcher` method): + +```java + private static DispatchHandler dispatcher( + Map handlers, + List interceptors, + List decorators) { + return new DispatchHandler(handlers, interceptors, decorators, new ResponseRenderer(Map.of())); + } +``` + +- [ ] **Step 3: Write the failing test** + +Append this test method to the class: + +```java + @Test + void decoratorSeesInterceptorBoundScopedValue() throws Exception { + RequestHandler ok = req -> Response.status(HTTP_OK); + RequestInterceptor bindCid = + (request, next) -> ScopedValue.where(CORRELATION_ID, "cid-123").call(next::proceed); + ResponseDecorator stampCid = + (req, resp) -> resp.withHeader("X-Correlation-Id", CORRELATION_ID.get()); + + HttpExchange ex = stubExchange(); + AtomicReference rendered = new AtomicReference<>(); + + withRequest( + "get-x", + () -> { + dispatcher(Map.of("get-x", ok), List.of(bindCid), List.of(stampCid)).handle(ex); + rendered.set((Response) ex.getAttribute(DispatchHandler.RESPONSE_ATTR)); + return null; + }); + + assertThat(rendered.get().headers()).containsEntry("X-Correlation-Id", "cid-123"); + } +``` + +- [ ] **Step 4: Wire up `exchange.getAttribute` on the stub** + +The stub returned by `stubExchange()` is a Mockito mock and `getAttribute(...)` returns `null` by default. Switch the stub to capture the `setAttribute(...)` call. Replace the existing `stubExchange()` body with: + +```java + private static HttpExchange stubExchange() { + HttpExchange exchange = mock(HttpExchange.class); + when(exchange.getResponseHeaders()).thenReturn(new Headers()); + Map attrs = new HashMap<>(); + doAnswer( + inv -> { + attrs.put(inv.getArgument(0), inv.getArgument(1)); + return null; + }) + .when(exchange) + .setAttribute(anyString(), any()); + when(exchange.getAttribute(anyString())).thenAnswer(inv -> attrs.get(inv.getArgument(0))); + return exchange; + } +``` + +Add the imports the new stub needs (keep alphabetical): + +```java +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doAnswer; +import java.util.HashMap; +``` + +- [ ] **Step 5: Run the new test and confirm it FAILS for the right reason** + +Run: + +```bash +mvn test -Dtest=DispatchHandlerTest#decoratorSeesInterceptorBoundScopedValue +``` + +Expected: test fails with `java.util.NoSuchElementException: ScopedValue not bound` thrown from `stampCid` — proves the bug. + +If it fails for any other reason (compile error, NPE in the stub, etc.), stop and fix that before continuing — the test must fail *because the bug exists*, not because the test is broken. + +- [ ] **Step 6: Commit the failing test** + +```bash +git add src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java +git commit -m "test: Decorator should see interceptor-bound ScopedValue" +``` + +(Pre-commit will run Google Java Format; let it.) + +--- + +## Task 2: Fix — move decorator loop inside the interceptor scope + +**Files:** +- Modify: `src/main/java/com/retailsvc/http/internal/DispatchHandler.java` + +- [ ] **Step 1: Move the decorator loop into `invoke`'s base case** + +Open `src/main/java/com/retailsvc/http/internal/DispatchHandler.java`. Replace the current `handle` and `invoke` methods with: + +```java + @Override + public void handle(HttpExchange exchange) throws IOException { + Request request = CURRENT.get(); + RequestHandler handler = handlers.get(request.operationId()); + Response response = invoke(0, request, handler); + exchange.setAttribute(RESPONSE_ATTR, response); + renderer.render(exchange, response); + } + + private Response invoke(int idx, Request request, RequestHandler handler) { + if (idx == interceptors.size()) { + Response response = handler.handle(request); + for (ResponseDecorator decorator : decorators) { + response = decorator.decorate(request, response); + } + return response; + } + return interceptors.get(idx).around(request, () -> invoke(idx + 1, request, handler)); + } +``` + +The change: the `for (ResponseDecorator ...)` loop moved from `handle` into the `idx == interceptors.size()` branch of `invoke`. Nothing else in the file changes. + +- [ ] **Step 2: Run the previously-failing test and confirm it PASSES** + +Run: + +```bash +mvn test -Dtest=DispatchHandlerTest#decoratorSeesInterceptorBoundScopedValue +``` + +Expected: PASS. + +- [ ] **Step 3: Run the whole `DispatchHandlerTest` class** + +Run: + +```bash +mvn test -Dtest=DispatchHandlerTest +``` + +Expected: all tests PASS — pre-existing `invokesRegisteredHandler` should still pass; the new test passes. + +- [ ] **Step 4: Commit the fix** + +```bash +git add src/main/java/com/retailsvc/http/internal/DispatchHandler.java +git commit -m "fix: Run response decorators inside interceptor ScopedValue scope" +``` + +--- + +## Task 3: Pin interceptor-observes-decorated-response + +**Files:** +- Modify: `src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java` + +- [ ] **Step 1: Add the test** + +Append to the class: + +```java + @Test + void interceptorObservesDecoratedResponse() throws Exception { + RequestHandler ok = req -> Response.status(HTTP_OK); + AtomicReference seen = new AtomicReference<>(); + RequestInterceptor capture = + (request, next) -> { + Response r = next.proceed(); + seen.set(r); + return r; + }; + ResponseDecorator stamp = (req, resp) -> resp.withHeader("X-Stamped", "yes"); + + HttpExchange ex = stubExchange(); + withRequest( + "get-x", + () -> { + dispatcher(Map.of("get-x", ok), List.of(capture), List.of(stamp)).handle(ex); + return null; + }); + + assertThat(seen.get()).isNotNull(); + assertThat(seen.get().headers()).containsEntry("X-Stamped", "yes"); + } +``` + +- [ ] **Step 2: Run the test** + +```bash +mvn test -Dtest=DispatchHandlerTest#interceptorObservesDecoratedResponse +``` + +Expected: PASS. + +- [ ] **Step 3: Commit** + +```bash +git add src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java +git commit -m "test: Interceptor observes decorated response" +``` + +--- + +## Task 4: Pin decorator-throw-is-catchable-by-interceptor + +Adds `import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;` alongside the existing `HTTP_OK` static import (memory: `feedback_http_status_constants` — no magic numbers). + +**Files:** +- Modify: `src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java` + +- [ ] **Step 1: Add the test** + +Append to the class: + +```java + @Test + void interceptorCanCatchDecoratorFailure() throws Exception { + RequestHandler ok = req -> Response.status(HTTP_OK); + AtomicBoolean caught = new AtomicBoolean(false); + RequestInterceptor catcher = + (request, next) -> { + try { + return next.proceed(); + } catch (RuntimeException e) { + caught.set(true); + return Response.status(HTTP_INTERNAL_ERROR); + } + }; + ResponseDecorator boom = + (req, resp) -> { + throw new IllegalStateException("boom"); + }; + + HttpExchange ex = stubExchange(); + withRequest( + "get-x", + () -> { + dispatcher(Map.of("get-x", ok), List.of(catcher), List.of(boom)).handle(ex); + return null; + }); + + assertThat(caught.get()).isTrue(); + } +``` + +- [ ] **Step 2: Run the test** + +```bash +mvn test -Dtest=DispatchHandlerTest#interceptorCanCatchDecoratorFailure +``` + +Expected: PASS. (Before the Task 2 fix this would have failed because the decorator exception propagated past the already-popped interceptor frame straight to `ExceptionFilter`; after the fix the throw happens *inside* the interceptor's `try`.) + +- [ ] **Step 3: Commit** + +```bash +git add src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java +git commit -m "test: Interceptor can catch decorator failures" +``` + +--- + +## Task 5: Full verification + +**Files:** none (verification only). + +- [ ] **Step 1: Run the full unit suite** + +```bash +mvn test +``` + +Expected: BUILD SUCCESS, zero failures, zero errors. No flakiness expected — all new tests are synchronous and use the existing `withRequest` ScopedValue harness. + +- [ ] **Step 2: Run the integration suite** + +```bash +mvn verify +``` + +Expected: BUILD SUCCESS. This runs Failsafe IT tests including `DecoratorAndInterceptorIT`, which exercises the same code path end-to-end through `HttpServer`. Confirms no regression in the existing decorator + interceptor IT. + +- [ ] **Step 3: Inspect the JaCoCo report for `DispatchHandler`** + +Open `target/site/jacoco/com.retailsvc.http.internal/DispatchHandler.html`. Confirm both branches of `invoke` (`idx == interceptors.size()` and the recursive case) are covered and the new decorator loop inside the base case is exercised. If a branch is uncovered, add a test before continuing. + +- [ ] **Step 4: SonarLint pre-push check** + +Per project memory `feedback_sonar_pre_push`, analyze every touched file with SonarLint MCP before pushing. The worktree caveat from `feedback_sonarlint_blind_to_worktrees` applies — SonarLint may return `not_found` for files only in the worktree. If so, rely on the CI scan for this branch and continue; do not skip the local check on files that *are* visible. + +Run SonarLint MCP against: +- `src/main/java/com/retailsvc/http/internal/DispatchHandler.java` +- `src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java` + +Fix any new issues in this branch before pushing. + +- [ ] **Step 5: Push and open PR** + +```bash +git push -u origin fix/decorator-scoped-values +gh pr create --title "fix: Run response decorators inside interceptor ScopedValue scope" --body "$(cat <<'EOF' +## Summary +- Move the response-decorator loop into the base case of `DispatchHandler.invoke(...)` so decorators execute inside every `RequestInterceptor`'s `ScopedValue` scope. Honors the contract documented in README "Combining the two". +- Interceptors observe the decorated `Response` on the way back up the stack. +- A decorator exception now propagates through the interceptor chain (interceptors can catch it) before reaching `ExceptionFilter`. + +Spec: `docs/superpowers/specs/2026-05-23-decorator-scoped-value-scope-design.md` + +## Test plan +- [x] New unit test: decorator reads interceptor-bound `ScopedValue` +- [x] New unit test: interceptor sees decorator-added header on the returned `Response` +- [x] New unit test: interceptor `try`/`catch` around `next.proceed()` catches a throwing decorator +- [x] `mvn verify` passes (including `DecoratorAndInterceptorIT`) +EOF +)" +``` + +--- + +## Notes for the implementer + +- **Do not touch `RequestPreparationFilter` or `AfterResponseHook`.** The spec explicitly scopes those out. Fixing after-hooks to see interceptor ScopedValues is a separate, larger change with a contract shift. +- **Do not edit `README.md`.** The README is already correct; this PR brings the implementation into compliance. +- **Decorator order is preserved.** They still iterate in registration order; that was true before and is still true after the move. +- **Branch is `fix/decorator-scoped-values`** (already created via worktree). +- **Commit subjects start with a capital letter after the type prefix** — see memory `feedback_skip_commitlint_in_worktrees`. From 9b0803512e55ed013fd6e486e54b0f257b93cd46 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Sat, 23 May 2026 00:35:45 +0200 Subject: [PATCH 03/11] test: Decorator should see interceptor-bound ScopedValue --- .../http/internal/DispatchHandlerTest.java | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java index acd6dbd..4eabe8b 100644 --- a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java +++ b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java @@ -2,24 +2,42 @@ import static java.net.HttpURLConnection.HTTP_OK; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import com.retailsvc.http.Request; import com.retailsvc.http.RequestHandler; +import com.retailsvc.http.RequestInterceptor; import com.retailsvc.http.Response; +import com.retailsvc.http.ResponseDecorator; import com.sun.net.httpserver.Headers; import com.sun.net.httpserver.HttpExchange; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.Test; class DispatchHandlerTest { + private static final ScopedValue CORRELATION_ID = ScopedValue.newInstance(); + private static HttpExchange stubExchange() { HttpExchange exchange = mock(HttpExchange.class); when(exchange.getResponseHeaders()).thenReturn(new Headers()); + Map attrs = new HashMap<>(); + doAnswer( + inv -> { + attrs.put(inv.getArgument(0), inv.getArgument(1)); + return null; + }) + .when(exchange) + .setAttribute(anyString(), any()); + when(exchange.getAttribute(anyString())).thenAnswer(inv -> attrs.get(inv.getArgument(0))); return exchange; } @@ -27,6 +45,13 @@ private static DispatchHandler dispatcher(Map handlers) return new DispatchHandler(handlers, List.of(), List.of(), new ResponseRenderer(Map.of())); } + private static DispatchHandler dispatcher( + Map handlers, + List interceptors, + List decorators) { + return new DispatchHandler(handlers, interceptors, decorators, new ResponseRenderer(Map.of())); + } + private static void withRequest(String operationId, ScopedValue.CallableOp body) throws Exception { Request req = new Request(new byte[0], null, null, operationId, Map.of(), null, n -> null); @@ -52,4 +77,26 @@ void invokesRegisteredHandler() throws Exception { assertThat(called.get()).isTrue(); } + + @Test + void decoratorSeesInterceptorBoundScopedValue() throws Exception { + RequestHandler ok = req -> Response.status(HTTP_OK); + RequestInterceptor bindCid = + (request, next) -> ScopedValue.where(CORRELATION_ID, "cid-123").call(next::proceed); + ResponseDecorator stampCid = + (req, resp) -> resp.withHeader("X-Correlation-Id", CORRELATION_ID.get()); + + HttpExchange ex = stubExchange(); + AtomicReference rendered = new AtomicReference<>(); + + withRequest( + "get-x", + () -> { + dispatcher(Map.of("get-x", ok), List.of(bindCid), List.of(stampCid)).handle(ex); + rendered.set((Response) ex.getAttribute(DispatchHandler.RESPONSE_ATTR)); + return null; + }); + + assertThat(rendered.get().headers()).containsEntry("X-Correlation-Id", "cid-123"); + } } From fb58715ed25b1ea394bde5f9fcd191aebdd8b747 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Sat, 23 May 2026 00:39:10 +0200 Subject: [PATCH 04/11] fix: Run response decorators inside interceptor ScopedValue scope --- .../com/retailsvc/http/internal/DispatchHandler.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/retailsvc/http/internal/DispatchHandler.java b/src/main/java/com/retailsvc/http/internal/DispatchHandler.java index fceab90..d506722 100644 --- a/src/main/java/com/retailsvc/http/internal/DispatchHandler.java +++ b/src/main/java/com/retailsvc/http/internal/DispatchHandler.java @@ -37,16 +37,17 @@ public void handle(HttpExchange exchange) throws IOException { Request request = CURRENT.get(); RequestHandler handler = handlers.get(request.operationId()); Response response = invoke(0, request, handler); - for (ResponseDecorator decorator : decorators) { - response = decorator.decorate(request, response); - } exchange.setAttribute(RESPONSE_ATTR, response); renderer.render(exchange, response); } private Response invoke(int idx, Request request, RequestHandler handler) { if (idx == interceptors.size()) { - return handler.handle(request); + Response response = handler.handle(request); + for (ResponseDecorator decorator : decorators) { + response = decorator.decorate(request, response); + } + return response; } return interceptors.get(idx).around(request, () -> invoke(idx + 1, request, handler)); } From 579c127a02520417613f19dd6a088e6d4ddb8559 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Sat, 23 May 2026 00:41:33 +0200 Subject: [PATCH 05/11] docs: Document ScopedValue visibility in ResponseDecorator javadoc --- src/main/java/com/retailsvc/http/ResponseDecorator.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/com/retailsvc/http/ResponseDecorator.java b/src/main/java/com/retailsvc/http/ResponseDecorator.java index 5603a98..266bf64 100644 --- a/src/main/java/com/retailsvc/http/ResponseDecorator.java +++ b/src/main/java/com/retailsvc/http/ResponseDecorator.java @@ -8,6 +8,9 @@ *

Because decorators run after the handler, decorator-supplied headers override * handler-supplied ones on conflict. If you need the opposite semantics, use {@link * Response#withHeaders(java.util.Map)} inside the handler instead. + * + *

Decorators run inside the {@link RequestInterceptor} chain, so any {@link ScopedValue} + * bindings established by interceptors are visible here. */ @FunctionalInterface public interface ResponseDecorator { From 09d5ef26243ee3df082bbe01a826124c17bb852e Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Sat, 23 May 2026 00:42:20 +0200 Subject: [PATCH 06/11] test: Interceptor observes decorated response --- .../http/internal/DispatchHandlerTest.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java index 4eabe8b..5d2568b 100644 --- a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java +++ b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java @@ -99,4 +99,28 @@ void decoratorSeesInterceptorBoundScopedValue() throws Exception { assertThat(rendered.get().headers()).containsEntry("X-Correlation-Id", "cid-123"); } + + @Test + void interceptorObservesDecoratedResponse() throws Exception { + RequestHandler ok = req -> Response.status(HTTP_OK); + AtomicReference seen = new AtomicReference<>(); + RequestInterceptor capture = + (request, next) -> { + Response r = next.proceed(); + seen.set(r); + return r; + }; + ResponseDecorator stamp = (req, resp) -> resp.withHeader("X-Stamped", "yes"); + + HttpExchange ex = stubExchange(); + withRequest( + "get-x", + () -> { + dispatcher(Map.of("get-x", ok), List.of(capture), List.of(stamp)).handle(ex); + return null; + }); + + assertThat(seen.get()).isNotNull(); + assertThat(seen.get().headers()).containsEntry("X-Stamped", "yes"); + } } From 6fceecbb157ae8597353fb2518f3e2d78b76c46a Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Sat, 23 May 2026 00:43:51 +0200 Subject: [PATCH 07/11] test: Interceptor can catch decorator failures --- .../http/internal/DispatchHandlerTest.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java index 5d2568b..cf3f5b7 100644 --- a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java +++ b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java @@ -1,5 +1,6 @@ package com.retailsvc.http.internal; +import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR; import static java.net.HttpURLConnection.HTTP_OK; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; @@ -123,4 +124,33 @@ void interceptorObservesDecoratedResponse() throws Exception { assertThat(seen.get()).isNotNull(); assertThat(seen.get().headers()).containsEntry("X-Stamped", "yes"); } + + @Test + void interceptorCanCatchDecoratorFailure() throws Exception { + RequestHandler ok = req -> Response.status(HTTP_OK); + AtomicBoolean caught = new AtomicBoolean(false); + RequestInterceptor catcher = + (request, next) -> { + try { + return next.proceed(); + } catch (RuntimeException e) { + caught.set(true); + return Response.status(HTTP_INTERNAL_ERROR); + } + }; + ResponseDecorator boom = + (req, resp) -> { + throw new IllegalStateException("boom"); + }; + + HttpExchange ex = stubExchange(); + withRequest( + "get-x", + () -> { + dispatcher(Map.of("get-x", ok), List.of(catcher), List.of(boom)).handle(ex); + return null; + }); + + assertThat(caught.get()).isTrue(); + } } From 84688bf0df23dc3b96ed8f899d8f8577357de07e Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Sat, 23 May 2026 00:52:29 +0200 Subject: [PATCH 08/11] docs: Note decorator exception propagation in README --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 30e9bc4..790df12 100644 --- a/README.md +++ b/README.md @@ -577,7 +577,9 @@ OpenApiServer.builder() Decorators run inside the interceptor's `ScopedValue` binding (the decorator transforms the `Response` returned by `next.proceed()`, which is still on the call stack), so -`CORRELATION_ID.get()` / `TENANT_ID.get()` see the bound values. +`CORRELATION_ID.get()` / `TENANT_ID.get()` see the bound values. If a decorator throws, the +exception propagates through any wrapping interceptors before reaching the `ExceptionFilter`, so +interceptors that catch around `next.proceed()` will observe decorator failures. A handler in this setup is just business logic: From a79a35de9f3c3db75d0b3dec48db2f754bc0239c Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Sat, 23 May 2026 00:52:38 +0200 Subject: [PATCH 09/11] test: Add IT for decorator reading interceptor-bound ScopedValue --- .../http/DecoratorAndInterceptorIT.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/test/java/com/retailsvc/http/DecoratorAndInterceptorIT.java b/src/test/java/com/retailsvc/http/DecoratorAndInterceptorIT.java index ac42437..dbcbfff 100644 --- a/src/test/java/com/retailsvc/http/DecoratorAndInterceptorIT.java +++ b/src/test/java/com/retailsvc/http/DecoratorAndInterceptorIT.java @@ -66,6 +66,24 @@ void interceptorBindsScopedValueVisibleToHandler() throws Exception { assertThat(call("/api/v1/data").body()).isEqualTo("acme"); } + @Test + void decoratorReadsInterceptorBoundScopedValue() throws Exception { + RequestHandler ok = req -> Response.text(HTTP_OK, "ok"); + server = + newBuilder() + .spec(spec) + .handlers(stubAllHandlers(Map.of("get-data", ok, "post-data", ok))) + .interceptor((request, next) -> ScopedValue.where(TENANT, "acme").call(next::proceed)) + .responseDecorator((req, resp) -> resp.withHeader("X-Tenant-Id", TENANT.get())) + .port(0) + .build(); + + var resp = call("/api/v1/data"); + + assertThat(resp.statusCode()).isEqualTo(HTTP_OK); + assertThat(resp.headers().firstValue("X-Tenant-Id")).contains("acme"); + } + @Test void interceptorsRunInRegistrationOrder() throws Exception { List trace = new CopyOnWriteArrayList<>(); From 508198df92d34e8154152406f9ec49d711d18cf8 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Sat, 23 May 2026 00:52:45 +0200 Subject: [PATCH 10/11] test: Assert response status in decorator-throw interceptor catch test --- .../java/com/retailsvc/http/internal/DispatchHandlerTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java index cf3f5b7..eb4595b 100644 --- a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java +++ b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java @@ -152,5 +152,8 @@ void interceptorCanCatchDecoratorFailure() throws Exception { }); assertThat(caught.get()).isTrue(); + Response rendered = (Response) ex.getAttribute(DispatchHandler.RESPONSE_ATTR); + assertThat(rendered).isNotNull(); + assertThat(rendered.status()).isEqualTo(HTTP_INTERNAL_ERROR); } } From a6da08cc8cb22d1e771bf675ad558e6a32951480 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Sat, 23 May 2026 00:58:46 +0200 Subject: [PATCH 11/11] fix: Use unnamed pattern variable in unused catch binding --- .../java/com/retailsvc/http/internal/DispatchHandlerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java index eb4595b..2c03873 100644 --- a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java +++ b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java @@ -133,7 +133,7 @@ void interceptorCanCatchDecoratorFailure() throws Exception { (request, next) -> { try { return next.proceed(); - } catch (RuntimeException e) { + } catch (RuntimeException _) { caught.set(true); return Response.status(HTTP_INTERNAL_ERROR); }