leizhiyuan opened a new pull request, #1420: URL: https://github.com/apache/rocketmq-clients/pull/1420
### Which Issue(s) This PR Fixes Fixes #1418 ### Brief Description **Scenario.** A lite topic usually comes from business data, such as a session or device id. When that value is missing, the application builds a lite message with `""` or `" "`. Today `build()` succeeds, and the send fails on the server: - An empty string loses its lite semantics on the wire, because protobuf does not distinguish an empty `lite_topic` from an unset one. The server reports a misleading `40014 ... expected type is LITE, but actual type is NORMAL`. - Whitespace gets `40020 lite topic cannot be the char sequence of whitespace`. **Change.** This aligns with Java, where `MessageBuilderImpl#setLiteTopic` checks `checkArgument(StringUtils.isNotBlank(liteTopic), "liteTopic should not be blank")`. - `MessageBuilder::build()` now fails with `ErrorKind::InvalidMessage`: `lite_topic should not be blank.` when the lite topic is empty or whitespace only. This applies to both `lite_message_builder(...)` and `set_lite_topic(...)`. - Rust setters return `Self` and cannot fail, so the check lives in `check_message()`, next to the existing lite checks (`lite_topic and message_group can not be set at the same time`, ...). Java fails in the setter itself. The observable difference is only *when* the error is raised, not *whether*. - A non-blank lite topic with surrounding whitespace, such as `" lite_topic "`, is kept as is, like Java. Java does not trim either. - The doc comments of `lite_message_builder` and `set_lite_topic` mention the constraint. #### Notes for reviewers - **No conflicts.** Only `rust/src/model/message.rs` is touched. `git merge-tree` against the open Rust PRs #1381, #1404, #1407, #1409, #1411, #1414, #1416 and __B10_PR__ (the access url prefix fix) is clean. Overview in #1412. - **Behavior change.** Code that built a lite message with a blank lite topic used to get `Ok` from `build()` and an error from the server. It now gets the error from `build()`. Such a message could never be sent successfully, so no working usage is affected. ### How Did You Test This Change? **Unit test.** `test_build_message_with_blank_lite_topic` fails on `master`. It checks that `""`, `" "` and `"\t\n "` are rejected through both `lite_message_builder` and `set_lite_topic` with `InvalidMessage` and the message above, and that `" lite_topic "` is accepted and kept as is. ``` $ cargo test test result: ok. 90 passed; 0 failed; 1 ignored (lib; master: 89) test result: ok. 3 passed; 0 failed (lite_integration_test) test result: ok. 2 passed; 0 failed; 3 ignored (doc-tests) $ cargo fmt --check # clean $ RUSTDOCFLAGS="--cfg docsrs" cargo doc --no-deps --all-features # no warnings $ cargo build --examples # ok ``` `cargo clippy --all-features -- -D warnings` reports only the 2 `for_kv_map` errors that `master` already has in `rust/src/client.rs` (lines 904 and 912). `cargo clippy --lib --tests` reports the same findings as `master`. **End-to-end.** I ran this against a managed RocketMQ 5 server: | lite topic | before | after | |---|---|---| | `""` | `build()` Ok, `send()` fails on the server with `40014` (expected LITE, actual NORMAL) | `build()` fails: `lite_topic should not be blank.` | | `" "` | `build()` Ok, `send()` fails on the server with `40020` | `build()` fails: `lite_topic should not be blank.` | The other lite producer scenarios (mutual exclusion checks, length 64, special characters, ...) behave the same as before. --- Tracked in #1412 (overview of the related Rust issues / PRs and a suggested merge order). -- 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]
