Repository navigation
CAMEL-25745: camel-http - make it stand out that a message with a body is sent as a POST unless httpMethod is set - #28041
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet-bot
left a comment
There was a problem hiding this comment.
LGTM. Clean documentation enhancement and error-message improvement for the body-as-POST surprise in camel-http.
The mandatoryBodyStream static→instance refactor is safe (private method, single call site). The methodFromBody detection logic is correct — the new branch gives users targeted guidance based on whether the method was inferred from the body or set explicitly. Test coverage for both branches is solid.
One minor nit below (not a blocker).
This review was generated by an AI agent, Hermès on behalf of @gnodet.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 76 of 706 tested, 26 compile-only — current: 76 all testedMaveniverse Scalpel detected 76 affected modules (current approach: 76). Skip-tests mode would test 76 modules (2 direct + 75 downstream), skip tests for 26 (generated code, meta-modules) Modules Scalpel would test (76)
Modules with tests skipped (26)
Build reactor — dependencies compiled but only changed modules were tested (2 modules, 20.3s total)Total reactor time: 20.3s
Top 20 slowest modules:
|
…y is sent as a POST unless httpMethod is set The http page opens its Usage section with which HTTP method is used: the rule, the trap after unmarshal, split or setBody, the fix (httpMethod GET; a GET sends no body unless getWithBody is true), the order, and examples in Java, XML and YAML. camel_catalog_doc returns the start of the page, so the rule is in its answers too. How to set the http method names the option first, and its YAML example is canonical. The error for a Map or List body says, when neither the option nor the header set the method, that the call is a POST because the message has a body, and how to make it a GET. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STT6whBgK1AqsSsUKrnE8m
…other examples on the page Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STT6whBgK1AqsSsUKrnE8m
db50ef5 to
dd81ea0
Compare
gnodet-bot
left a comment
There was a problem hiding this comment.
Previous findings are all addressed. LGTM.
The nit on the YAML setHeader compact form — ✅ addressed in both .adoc copies (commit dd81ea07).
The static → instance change on mandatoryBodyStream is safe (private method, single call site). The error-message branching correctly mirrors the actual POST-by-body decision in HttpProducer — the new hint fires only when neither httpMethod nor CamelHttpMethod is set, which is exactly the ambiguous case (CAMEL-25745). Both branches have test coverage.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
CAMEL-25745: without the
httpMethodoption or theCamelHttpMethodheader, the http producer picks the method from the message body: a message with a body is sent as a POST, a message without one as a GET. Afterunmarshal,splitorsetBodythe message has a body, so a call meant as a GET is sent as a POST. This trips many users, and AI models too: in the overnight benchmark of Oct 17-18, a step that calls a stock service for each line of a split order failed 6 of 10 times on it. The behaviour cannot change without breaking existing applications, so this PR makes the rule stand out.What
getWithBody.httpMethod=GET, and a POST of JSON withhttpMethod=POST.camel_catalog_docreturns the start of a component's page, so the rule is now in every answer about http.httpMethodoption first, then the header.BodyFactsmessage (what the body is, the step that set it, and the marshal that writes it) now says "The call is a POST because the message has a body and neither the httpMethod option nor the CamelHttpMethod header sets the method: for a GET, set httpMethod GET on the endpoint (a GET sends no body); for a POST, marshal it first ...". With an explicit method the message stays as on main.The catalog copy of the page is updated.
Checks
camel runin all three DSLs, against a small stub: the stock lookup arrives as a GET with a Map body in the message, and the order arrives as a POST with JSON. The same split route withouthttpMethodfails with the Map body error, as in the benchmark.HttpProducerMapBodyTest: the new test, and the explicit-POST test checks that the GET advice is left out.HttpMethodsTestpasses.BodyFacts): the GET/POST advice now sits on top of theBodyFactsmessage, andHttpProducerMapBodyTestpasses again (4 tests).🤖 Generated with Claude Code
https://claude.ai/code/session_01STT6whBgK1AqsSsUKrnE8m