Croway commented on PR #26203:
URL: https://github.com/apache/camel/pull/26203#issuecomment-5616690147
The SPI here looks good, but I think it can't take effect yet: the branch it
plugs into is never reached on the supported runtimes.
I built this branch and registered a custom
`RestOpenApiUnmatchedRequestHandler`, then sent requests that match no
operation:
| | `POST /api/v3/no-such-op` | `PATCH /api/v3/pet/123` | handler
invocations |
|---|---|---|---|
| `camel-platform-http-vertx` (Camel Main) | 404, Vert.x's HTML page | 405,
empty | **0** |
| `camel-platform-http-starter` (Spring Boot) | 404, Spring's
`{"timestamp":…,"error":"Not Found"}` | 405, Spring error JSON | **0** |
The three runtimes have only two consumer implementations: camel-quarkus
reuses `camel-platform-http-vertx` verbatim (`PlatformHttpRecorder`
instantiates `VertxPlatformHttpEngine` over a Quarkus-managed
`VertxPlatformHttpRouter`), so it shares the vertx code path above. I have not
measured Quarkus — that row is inferred from the shared code, not tested.
Both implementations register only the exact OAS surface, so the HTTP layer
rejects anything else before Camel is involved:
- Vert.x: `VertxPlatformHttpConsumer.startRestServicesContractFirst()`
creates one route per (path, method) and returns early, so no base-path
catch-all exists (since 4.18.0, CAMEL-22971).
- Spring Boot: `DefaultRestOpenapiProcessorStrategy.validateOpenApi()` calls
`addHttpEndpoint(uri, verbs, …)` per OAS path, which
`CamelRequestHandlerMapping` turns into `RequestMappingInfo`s restricted to
those verbs — an unknown path/verb becomes Spring MVC's 404/405.
So `RestOpenApiProcessor:137` `unmatchedRequestHandler.handle(...)` is
currently dead code. The PR's tests don't catch this because they drive the
processor directly against a mocked `PlatformHttpComponent`.
The good news is the missing half is small, and it doesn't need any change
to what you've written here. I prototyped it on top of this branch:
- **Spring Boot** — one extra registration in
`DefaultRestOpenapiProcessorStrategy.validateOpenApi()`:
`phc.addHttpEndpoint(basePath, null, null, null, consumer)`. The endpoint
already has `matchOnUriPrefix=true`, so `CamelRequestHandlerMapping` rewrites
it to `/api/v3/{*matchOnUriPrefix}`, and Spring's pattern-specificity ordering
keeps the per-operation mappings winning. (Registering `basePath + "/**"`
instead does *not* work — the rewrite appends `/{*matchOnUriPrefix}` and
produces an invalid `PathPattern`, silently.)
- **Vert.x (Camel Main, and Quarkus by the same code)** — drop the early
`return` in `VertxPlatformHttpConsumer.doStart():163` and fall through to the
existing registration at line 169, which already builds `router.route(path)`
(path ends in `*`) and applies `configureSecurityHandler`. Vert.x matches in
registration order, so the contract-first routes still win.
With that, on both tested runtimes: matched operations unchanged (200), and
unmatched requests reach the handler — 404/405 with the custom body,
invocations 2. As a bonus Camel's `Allow` is more accurate than Spring's: `GET,
POST, DELETE` for `/pet/{petId}`, where Spring answered `PUT, GET, DELETE,
POST` (PUT is only declared on `/pet` and leaked in via the prefix pattern).
To keep this opt-in I'd gate it on a new endpoint option, which also gives
users the choice explicitly:
```java
@UriParam(label = "consumer,advanced", defaultValue = "platform", enums =
"platform,camel",
description = "Who answers requests matching no operation in the
OAS: the HTTP layer "
+ "(Spring Boot's error controller, the Vert.x
router) or Camel via the unmatchedRequestHandler.")
private String unmatchedRequestHandling = "platform";
```
Default `platform` preserves today's behaviour, so no upgrade-guide entry is
needed. I'd avoid auto-enabling it just because a handler bean is present —
that would silently change 404 bodies on upgrade.
Two smaller notes on the current diff:
1. **The docs' "empty body" claim** (`rest-openapi-component.adoc:205`,
repeated at 211). `DefaultRestOpenApiUnmatchedRequestHandler.handle()` sets the
status code and `Allow` but never clears the body, and at that point
`exchange.getMessage()` is still the inbound request. Once the branch becomes
reachable this is worth re-checking with a request that has a payload — an
`exchange.getMessage().setBody(null)` in the default handler is probably
wanted. The existing test passes only because it never sets a request body.
2. **Multiple registered handlers fall back silently.**
`CamelContextHelper.findSingleByType` returns `null` unless exactly one bean
matches, so two registered handlers are ignored with no WARN. Also, the doc at
217-218 ("if two or more beans of this type are found in the registry, the
default handler is used") isn't accurate when a
`META-INF/services/.../rest-openapi-unmatched-request-handler-factory` file is
also on the classpath — the factory-finder handler wins there, not the default.
Happy to share the probe tests if useful.
_Claude Code on behalf of @Croway_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
