leizhiyuan opened a new pull request, #1439: URL: https://github.com/apache/rocketmq-clients/pull/1439
### Which Issue(s) This PR Fixes Fixes #1438 ### Brief Description **Scenario.** A `MessageView` can have no receipt handle. `receipt_handle` is optional in the protocol, and the proxy only sets it for popped messages. The message given to the transaction checker is one example. Calling `AckMessageEntry::receipt_handle()` on such a view, directly or through `SimpleConsumer::ack` / the push consumer's ack, nack and DLQ paths, panics inside the client. Details in #1438. **Change.** Align with Java: `MessageViewImpl#fromProtobuf` takes the handle from the protobuf getter (`""` when absent), and `getReceiptHandle()` never throws. - `MessageView::receipt_handle()` returns `unwrap_or_default()`, so an empty string instead of a panic. An ack with it is sent and rejected by the server, as in Java. A doc comment states this. #### Notes for reviewers - **Why not return `Option` / `Result`.** That would change the public `AckMessageEntry` trait, which users may implement for their own types (`SimpleConsumer::ack` takes `impl AckMessageEntry`). An empty string keeps the trait as is and matches Java. - **Conflicts.** `git merge-tree` is clean against every open Rust PR except #1431, which changes the same line. #1431 turns `receipt_handle` into a `Mutex<Option<String>>`, so the line reads `self.receipt_handle.lock().clone().unwrap()`. The resolution is `self.receipt_handle.lock().clone().unwrap_or_default()`; on that temporary merge, 92 passed. I also ran `cargo test --lib` on temporary merges with the PRs that touch `model/message.rs`: #1381 98 passed, #1404 99, #1420 91, #1429 98, #1437 92. Overview in #1412. ### How Did You Test This Change? **Unit test.** `model::message::tests::message_view_without_receipt_handle` builds a `MessageView` from a `pb::Message` without a receipt handle and expects `receipt_handle()` to be `""`. On `master` it panics. ``` $ cargo test -- --test-threads=1 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 $ cargo clippy --all-targets # same warnings as master $ cargo clippy --all-features -- -D warnings # only the 2 existing for_kv_map errors of master $ RUSTDOCFLAGS='-D warnings' cargo doc --no-deps # no warnings $ cargo build --examples --all-features # ok ``` **End-to-end.** Not run. Messages received by a consumer always carry a handle, and the realistic source without one is the transaction check, which needs a transaction topic that the server I use does not have. -- 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]
