davsclaus opened a new pull request, #28933: URL: https://github.com/apache/camel/pull/28933
Fixes [CAMEL-26155](https://issues.apache.org/jira/browse/CAMEL-26155). ### 1. waitForSendResult=false broke the async callback contract **What was wrong.** `RocketMQProducer.processInOnly` returned `!waitForSendResult`. With `waitForSendResult=false` (the default) it returned `true` (done synchronously) but called `callback.done(true)` later, from the RocketMQ `SendCallback` on the client thread. Camel continues routing from the callback, not from the return value, so in practice the producer always waited for the send result and failed the exchange when the send failed. The existing `RocketMQProducerSendFailureTest.testInOnlySendFailure` (from CAMEL-25272) relies on exactly that with the default setting. **What changed.** - `waitForSendResult=true`: unchanged (returns `false`, the callback sets the exception and calls `done(false)`). - `waitForSendResult=false`: calls `callback.done(true)` right after the send is started and returns `true`. The `SendCallback` only logs the result (DEBUG when ok, WARN when not ok or failed). It does not touch the exchange, which may be done already. - **The default of `waitForSendResult` is now `true`** (component and endpoint). Users who never set the option keep what they had in practice: the route waits for the send and a failed send fails the exchange. Only users who explicitly set `false` get the fire-and-forget behaviour the option describes. Implementing the proposal while keeping the default `false` would silently stop failing the exchange on a failed send for every user, and undo what CAMEL-25272 fixed. - The option javadoc says what each value does. **Tests.** `RocketMQProducerSendFailureTest`: - `testInOnlyWithoutWaitForSendResultIsDoneSynchronously`: calls the producer directly with `waitForSendResult=false` and asserts it returns `true` with the callback already called with `doneSync=true` on the calling thread. It fails without the fix (`expected: <true> but was: <null>`, or the callback ran on `AsyncSenderExecutor_1`). - `testInOnlySendFailureWithWaitForSendResult`: explicit `true` still fails the exchange. - `testInOnlySendFailure` (default) still fails the exchange, so it guards the new default. ### 2. Header names spelled CamelRockerMQ All 20 header values of `RocketMQConstants` are now `CamelRocketMQ...`. Per `design/headers.adoc` this is a hard rename with no deprecated aliases (the JIRA proposal suggested reading both names for a while; the design doc's policy is a hard break, so I followed that). The Java constant names are unchanged. The Endpoint DSL header accessors are renamed (`rockerMQTopic()` -> `rocketMQTopic()`, ...). `ImportantHeaderUtils` and `important-headers.json` list `CamelRocketMQKey`. The component doc examples use the new names. ### 3. The test-infra broker could not be reached from a Docker Desktop host `RocketMQBrokerContainer` registered the container IP with the name server, so on macOS the ITs failed with `RemotingConnectException: connect to 172.18.0.3:10911 failed` (reproduced on main). In random-port mode (tests) the broker now listens on a free host port, bound to the same port in the container, and registers `brokerIP1 = <docker host>`; the config is the classpath `broker1.conf` plus those two lines, copied into the container (same approach as `StrimziContainer` for Kafka). The fixed-port mode (`camel infra run`) is unchanged. ### 4. Tests skipped on aarch64 The rocketmq 5.3.3 image is published for arm64, so the `skipTests.aarch64` / `skipITs.aarch64` settings are removed. The s390x skip stays (no s390x image). **ITs run locally on an arm64 Mac (Docker Desktop):** `RocketMQRouteIT` and `RocketMQRequestReplyRouteIT` pass with this change; `RocketMQRouteIT` failed on main with the connect error above. Note that they are still disabled on CI by `@DisabledIfSystemProperty(named = "ci.env.name")` (CAMEL-19832); I did not change that. ### Docs and generated files - Upgrade guide 4.24: new `camel-rocketmq` section with the header table, the Endpoint DSL accessor rename, and the new `waitForSendResult` default. - Regenerated: component JSON, catalog JSON + doc, Endpoint DSL, Component DSL. ### Open question - The new default `waitForSendResult=true` is a deviation from the JIRA proposal (which kept `false` and would have changed the default behaviour). If you prefer keeping the default `false` (fire-and-forget by default), it is a one-line change plus the upgrade-guide wording, and `testInOnlySendFailure` would need `waitForSendResult=true`. Module tests: 11 unit tests and 2 ITs pass (`mvn -o clean install -Psourcecheck`, plus `mvn -o verify` with ITs). _Claude Code on behalf of Claus Ibsen (davsclaus)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
