camel-platform-http-starter: code review fixes (cookies, method restrict, exchange lifecycle, executors, ByteBuffer, unregister, SB4 idioms)

[Croway]

Description

This PR addresses the findings of a code review of camel-platform-http-starter. Each finding is an isolated commit so they can be reviewed (or reverted) independently:

1. Honor CookieHandler contract in SpringBootCookieHandler — getCookieValue returned the sentinel string "Cookie not found" and removeCookie returned the cookie name; the CookieHandler contract (and the Vert.x implementation) return the value, or null when the cookie does not exist.
2. Do not widen httpMethodRestrict to all methods on unparsable verbs — httpMethodRestrict=GET, POST (note the space) silently registered a mapping serving all HTTP methods because the unresolved token became a null RequestMethod. Tokens are now trimmed/upper-cased and an unknown verb fails closed. Also fixes a latent NPE (model.getVerbs() vs the local verbs variable).
3. Fix consumer exchange lifecycle — createExchange(true) combined with the manual releaseExchange(exchange, false) in afterProcess double-releases pooled exchanges (camel.main.exchange-factory=pooled); aligned with CamelServlet/VertxPlatformHttpConsumer (createExchange(false) + manual release). Also releases the exchange when request parsing or createUoW fails.
4. Fix consumer executor ownership — every consumer allocated a throwaway single-thread ExecutorService that was immediately overwritten; when the fallback executor was used it was never shut down; an engine created via the port-only constructor produced consumers that NPE'd on the first request.
5. Write only the ByteBuffer window in direct responses — bb.array() dumped the whole backing array (sliced/partially-consumed buffers leaked adjacent bytes; direct buffers threw). Also replaces the visibly decompiled doWriteDirectResponse body (var19 locals) with readable source re-aligned to DefaultHttpBinding.
6. Unregister Spring MVC mappings when a route stops — unregisterHttpEndpoint was a noop, so stopped/removed routes kept dispatching to a stopped consumer instead of returning 404. Mappings are now tracked per consumer, which keeps same-path/different-method consumers independent and sidesteps the shared-OPTIONS-handler issue noted in the code.
7. Align auto-configuration with Spring Boot 4 idioms — @AutoConfiguration for the imports-file-registered WebMvc configuration, Threading.VIRTUAL.isActive() instead of manual property parsing, ServerProperties instead of raw server.port parsing, type-safe after = references, no re-enabling of Boot-owned WebMvcProperties, specific exceptions. Executor selection semantics are unchanged.

Every fix comes with tests (new test classes: SpringBootPlatformHttpMethodRestrictTest, SpringBootPlatformHttpPooledExchangeTest, SpringBootPlatformHttpConsumerExecutorTest, SpringBootPlatformHttpByteBufferResponseTest, SpringBootPlatformHttpRouteLifecycleTest, plus extended cookie tests). Full mvn verify on the module passes.

Target

• main
• backport needed

Claude Code on behalf of Federico Mariani (@Croway)

🤖 Generated with Claude Code

[davsclaus]

Review Summary

All 7 fixes are correct, well-motivated, and backed by appropriate tests. Each fix is isolated in a separate commit, making review and potential reversion straightforward. CI passes.

Verified Correctness

1. Cookie handler contract — Verified against CookieHandler interface javadoc (null for missing, not "Cookie not found"). Matches the Vert.x implementation. removeCookie reads from request.getCookies() (incoming), so reading after adding the expiry cookie to the response is safe. Removal of @CookieValue annotation is correct — it's a Spring MVC handler annotation, not appropriate here.

2. Method restrict — model.getVerbs() was used instead of the local verbs variable (latent NPE). Untrimmed verbs ("GET, POST") produced null from RequestMethod.resolve(" POST"), silently widening to all methods. Fail-closed with IllegalArgumentException is the right behavior.

3. Exchange lifecycle — createExchange(true) + manual releaseExchange double-releases pooled exchanges. Changed to createExchange(false), consistent with both VertxPlatformHttpConsumer and CamelServlet in camel-core. The try/catch around setup + createUoW ensures the exchange is released on failure.

4. Executor ownership — Old 3-arg constructor created a throwaway Executors.newSingleThreadExecutor() immediately overwritten. Port-only engine constructor produced consumers with null executor (NPE). Now properly tracked with shutdownExecutor flag.

5. ByteBuffer response — bb.array() leaked adjacent bytes for sliced buffers and threw for direct buffers. The writeByteBuffer helper correctly handles both heap and direct buffers. The doWriteDirectResponse cleanup replaces what appears to be decompiled code with readable source aligned to DefaultHttpBinding.

6. Unregister mappings — unregisterHttpEndpoint was a noop. Per-consumer tracking with ConcurrentHashMap ensures stopping one route doesn't affect other consumers on the same path with different methods.

7. Spring Boot 4 idioms — Type-safe after = references, Threading.VIRTUAL.isActive(), ServerProperties, correct @AutoConfiguration usage.

Minor Observation (non-blocking)

In SpringBootPlatformHttpConsumer.doStop(), when shutdownExecutor is true (consumer-owned executor), the executor is shut down. If the route is later restarted (not just the context stopped), the same consumer would have a dead executor. This only affects the fallback path (SpringBootPlatformHttpEngine(port) without an executor), not the normal auto-configuration path. The current code on main never shuts down the executor (thread leak), so this PR is strictly better. Could be addressed as a follow-up if needed.

Test Coverage

5 new test classes plus 4 extended cookie tests cover all fixed scenarios including concurrent pooled exchanges, sliced/direct ByteBuffers, route stop/restart, and shared-path independence.

---

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.