CAMEL-24340: camel-salesforce: improve option and header documentation - #25323
Conversation
The apexMethod option was documented as "APEX method name", but the value is used verbatim as the HTTP verb of the Apex REST call. AbstractRestProcessor reads it, defaults it to GET, and passes it to RestClient.apexCall, whose parameter is named httpMethod and which feeds it straight into the HTTP request. A user following the documentation and supplying their actual Apex method name produces a malformed HTTP verb. The component doc page already described it correctly as the HTTP method, so the generated options table and the prose table contradicted each other on the same rendered page. The javadoc text is also what surfaces in IDE completion, the endpoint DSL builders, Camel JBang tooling and Spring Boot metadata, where the prose page is not visible at all. Improve the surrounding descriptions in the same pass, since they restated the option name without saying what the value controls: - apexUrl: a path relative to /services/apexrest/, not a URL. It can be supplied as the option, in the endpoint path, or via a header. - jobId: name the operations that need it, and that it covers Bulk API v1 and 2.0. - batchId: Bulk API v1 only; BulkApiV2Processor never reads it. - resultId: used only by getQueryResult, sourced from getQueryResultIds. - pubSubHost and pubSubPort: name the gRPC transport and the operations. - 15 Streaming API and Change Data Capture headers that followed a bare "The <name>." template. Most map to fields of the Salesforce ChangeEventHeader. Note that CamelSalesforceEventType carries two different values depending on the message kind. Documentation only. No behaviour change: this edits javadoc and @metadata descriptions and regenerates the component metadata. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2e87ff7 to
86ae4a4
Compare
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Good documentation improvement — the apexMethod fix alone prevents a real user pitfall (the old "APEX method name" description was misleading; the value is actually the HTTP verb, as confirmed by AbstractRestProcessor.java:637-640). The header descriptions for Streaming API / CDC are much more useful than the prior "The <name>." template.
Two minor observations (non-blocking):
-
defaultValue = "GET"on@UriParamforapexMethod: factually correct (matches the runtime default atAbstractRestProcessor:640), but note this is a metadata change — generated catalogs and tooling will now show a default value that was previously absent. Since it documents real behavior, this is fine. -
CamelSalesforceCreatedDate: the new description honestly notes the dual-type behavior (ZonedDateTime for platform events, String for PushTopic). The declaredjavaType = "java.time.ZonedDateTime"is a pre-existing inaccuracy that is correctly left out of scope here.
Generated files (catalog JSON, component DSL, endpoint DSL) are consistent with the source changes.
Note: this review covers project conventions and rules compliance. It does not replace specialized AI review tools (CodeRabbit, Sourcery) or static analyzers (SonarCloud).
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: 14 tested, 26 compile-only — current: 11 all testedMaveniverse Scalpel detected 40 affected modules (current approach: 11).
|
2 participants
JIRA: https://issues.apache.org/jira/browse/CAMEL-24340
The defect
apexMethodis documented as "APEX method name", but the component uses the value verbatim as the HTTP verb of the Apex REST call:AbstractRestProcessor:637reads the option, defaulting it toGETat:640:653passes it as the first argument torestClient.apexCall(...)RestClient:243declares that parameter asString httpMethod("HTTP method to execute")DefaultRestClient:499forwards it togetRequest(httpMethod, ...), andAbstractClientBase:169-171passes it into Jetty'sHttpRequest.method(String)There is no validation or lookup at any step. A user who follows the documentation and sets
apexMethodto their real Apex method name, for examplegetMerchandise, makes Camel issueGETMERCHANDISE /services/apexrest/..., which fails.The wording dates from CAMEL-8772 (2015) and has been propagated mechanically since.
Why this is not cosmetic: the hand-written Apex table already says "The HTTP method (e.g. GET, POST) to use", but the generated options table is included in the same page (
salesforce-component.adoc:58-60, about 175 lines above), so the page contradicts itself. More importantly the javadoc is what surfaces in IDE completion, the endpoint DSL builders, Camel JBang and Karavan tooling, and Spring Boot metadata, where the prose page is not visible at all.Other descriptions improved in the same pass
These restated the option name without saying what the value controls:
apexUrl/services/apexrest/, not a URL, and can be given as the option, in the endpoint path asapexCall/MyApexClass/, or viaCamelSalesforceApexUrljobIdbatchIdBulkApiV2Processornever reads itresultIdgetQueryResult, sourced fromgetQueryResultIdspubSubHost/pubSubPortPlus 15 Streaming API and Change Data Capture headers that followed a bare
"The <name>."template. Most map to fields of the SalesforceChangeEventHeader.Worth noting:
CamelSalesforceEventTypecarries two different values. For change and platform events it is the last segment of the configured topic name (StreamingApiConsumer:181,225); for PushTopic messages it is the Salesforce event type (:254). The description now covers both.Scope and testing
Documentation only. Every changed line is javadoc or
@Metadatatext, so there is no behaviour change and no test is applicable. Descriptions were derived by reading the call sites rather than from the option names, and claims that could not be verified in code (Salesforce id formats, an exhaustive list of Apex verbs) were deliberately left out.The regenerated metadata is included for the component, the catalog, and the generated Endpoint DSL and Component DSL builders, as is the convention for metadata changes.
Generated by Claude Code on behalf of @stn1slv