Fix contract-first REST DSL 404 when OpenAPI base path is root - #25752
Conversation
RestOpenApiHelper.determineBasePath() always falls back to
DEFAULT_BASE_PATH ("/") when the OpenAPI servers[0].url has no path
segment, so the effective base path is never truly empty. In
VertxPlatformHttpConsumer.startRestServicesContractFirst(), the Vert.x
route path was built by plain concatenation of basePath + baseUrl
("/" + "/hello" -> "//hello"), which Vert.x normalises away when
matching incoming requests, making every contract-first route
permanently unreachable whenever the contract's base path is root.
Normalise the join via a new buildNormalizedEndpoint() helper that
strips a trailing slash from the base path before concatenation, used
at both call sites (operation routes and the api-specification route).
Add regression tests covering a contract-first spec without a path
segment in servers[0].url.
Co-authored-by: Claude <noreply@anthropic.com>
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
This review focuses on rules/conventions (per project guidelines) and is not a substitute for CodeRabbit, Sourcery, SonarCloud, or similar specialized tools/static analysis.
Thanks for the fix and the clear write-up — the root cause analysis is spot on. I checked out the branch locally and verified it directly: reverting just the buildNormalizedEndpoint change reproduces the exact bug (404 instead of 200, "//pet/{petId}" instead of "/pet/{petId}"), and all 5 tests pass with the fix in place. Good, targeted regression coverage at both the unit level (buildNormalizedEndpoint) and the integration level (actual HTTP request through Vert.x).
Two minor, non-blocking notes below.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 18 tested, 29 compile-only — current: 18 all testedMaveniverse Scalpel detected 47 affected modules (current approach: 18).
|
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Correct fix for a real bug: when the OpenAPI base path is root ("/"), the contract-first REST DSL concatenated "/" + "/hello" → "//hello", which Vert.x normalized away, making every contract-first route permanently unreachable (404). The fix properly strips the trailing slash from basePath before concatenation via a new buildNormalizedEndpoint() helper, applied consistently to both call sites (operation routes and API specification routes).
Test coverage is solid — both focused unit tests (VertxPlatformHttpConsumerNormalizedEndpointTest) for the normalization logic and an end-to-end integration test (RestOpenApiContractFirstRootBasePathTest) that reproduces the actual 404. CI is green.
Minor observations (non-blocking):
- The new test files use
publicon classes/methods and JUnit assertions instead of AssertJ. The project convention recommends package-private visibility and AssertJ for new tests, but practically every other test in this module follows the samepublic/JUnit pattern, so this is consistent with existing module practice. - No JIRA ticket is linked — consider filing one so the commit message can follow the
CAMEL-XXXX:format.
📋 PR Metadata
| Aspect | Current | Suggested |
|---|---|---|
| Category | (unlabeled) | bug |
| Labels | components |
+ bug |
| Milestone | (none) | 4.22.0 |
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Guillaume Nodet
base can never be null here — both call sites only reach this method after target.equals(r.getBasePath()) matched a non-null target.
|
Removed the dead code.
|
RestOpenApiHelper.determineBasePath() always falls back to DEFAULT_BASE_PATH
("/") when the OpenAPI servers[0].url has no path segment, so the effective
base path is never truly empty. In
VertxPlatformHttpConsumer.startRestServicesContractFirst(), the Vert.x route
path was built by plain concatenation of basePath + baseUrl ("/" + "/hello"
-> "//hello"), which Vert.x normalizes away when matching incoming requests,
making every contract-first route permanently unreachable whenever the
contract's base path is root.
Normalize the join via a new buildNormalizedEndpoint() helper that strips a
trailing slash from the base path before concatenation, used at both call
sites (operation routes and the api-specification route). Adds regression
tests covering a contract-first spec without a path segment in
servers[0].url.
Backport of #25752 to camel-4.22.x.
Closes #25792
3 participants
RestOpenApiHelper.determineBasePath() always falls back to DEFAULT_BASE_PATH ("/") when the OpenAPI servers[0].url has no path segment, so the effective base path is never truly empty. In VertxPlatformHttpConsumer.startRestServicesContractFirst(), the Vert.x route path was built by plain concatenation of basePath + baseUrl ("/" + "/hello" -> "//hello"), which Vert.x normalizes away when matching incoming requests, making every contract-first route permanently unreachable whenever the contract's base path is root.
Normalize the join via a new buildNormalizedEndpoint() helper that strips a trailing slash from the base path before concatenation, used at both call sites (operation routes and the api-specification route).
Add regression tests covering a contract-first spec without a path segment in servers[0].url.
Description
The merge request fixes the problem, that Contract First APIs always return 404, if no base path has been set in
servers[0].url.Target
mainbranch)Tracking
Apache Camel coding standards and style
I checked that each commit in the pull request has a meaningful subject line and body.
I have run
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.AI-assisted contributions
Co-authored-bytrailers) and the PR description identifies the AI tool used.