[ISSUE #6326]♻️Refactor topic name handling in ConsumerStateGetter for improved clarity

[mxsm]

Which Issue(s) This PR Fixes(Closes)

• Fixes [Refactor♻️] Refactor topic name handling in ConsumerStateGetter for improved clarity #6326

Brief Description

How Did You Test This Change?

Summary by CodeRabbit

• Refactor
  • Optimized internal namespace handling and string management to improve performance and reduce unnecessary allocations.
  • Streamlined API parameter handling for improved flexibility.

Note: These are internal improvements with no visible changes to end-user functionality.

[rocketmq-rust-bot]

🔊@mxsm 🚀Thanks for your contribution🎉！

💡CodeRabbit(AI) will review your code first🔥！

Note

🚨The code review suggestions from CodeRabbit are to be used as a reference only, and the PR submitter can decide whether to make changes based on their own judgment. Ultimately, the project management personnel will conduct the final code review💥.

[coderabbitai]

Walkthrough

A refactoring initiative that modernizes string type handling across the RocketMQ codebase by transitioning from &str parameters to generic impl Into<CheetahString> inputs and CheetahString return types. This simplifies type conversions and reduces intermediate allocations in namespace-related operations.

Changes

Cohort / File(s)	Summary
Namespace Utility Refactoring rocketmq-remoting/src/protocol/namespace_util.rs	Core refactoring of wrap_namespace and wrap_namespace_and_retry functions to accept generic Into<CheetahString> parameters and return CheetahString instead of String. Adds early-return logic for empty inputs and updates internal string construction to use CheetahString::from_string.
Client Config and Producer API Updates rocketmq-client/src/base/client_config.rs, rocketmq-client/src/producer/default_mq_producer.rs	Updates public with_namespace method signatures to accept generic impl Into<CheetahString> instead of &str. Adds early-exit paths for empty or already-namespaced resources in client_config.rs.
Producer Implementation Simplification rocketmq-client/src/producer/producer_impl/default_mq_producer_impl.rs	Replaces explicit CheetahString::from_string wrapping of NamespaceUtil::wrap_namespace calls with direct usage of the utility function, eliminating redundant allocations.
Broker Runtime Cleanup rocketmq-broker/src/broker_runtime.rs	Simplifies ConsumerStateGetter online check by removing unnecessary CheetahString conversions for topic and group names; references are passed directly to find_subscription_data.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Strings once tangled, now refined,
CheetahString brings clarity to mind,
No more conversions left and right,
Namespace handling burns so bright!
The refactored path gleams with delight! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 5

❌ Failed checks (4 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The PR title references the specific refactoring objective (topic name handling in ConsumerStateGetter), but the actual changes extend far beyond this scope to include namespace utility refactoring and API signature changes across multiple files.	Update the PR title to accurately reflect the broader scope of changes, such as 'Refactor namespace handling and string type conversions across producer and remoting modules' to better represent the actual implementation.
Out of Scope Changes check	⚠️ Warning	The PR includes substantial changes beyond the stated objective of refactoring ConsumerStateGetter: refactoring NamespaceUtil with new generic signatures returning CheetahString, and updating public API signatures across multiple producer and client config modules.	Either link the broader namespace utility and API changes to separate issues, or revise the PR to focus solely on ConsumerStateGetter refactoring and address namespace changes in a separate PR.
Docstring Coverage	⚠️ Warning	Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection	⚠️ Warning	❌ Merge conflicts detected (6 files): ⚔️ rocketmq-broker/src/broker_runtime.rs (content) ⚔️ rocketmq-client/src/base/client_config.rs (content) ⚔️ rocketmq-client/src/producer/default_mq_producer.rs (content) ⚔️ rocketmq-client/src/producer/producer_impl/default_mq_producer_impl.rs (content) ⚔️ rocketmq-example/Cargo.toml (content) ⚔️ rocketmq-remoting/src/protocol/namespace_util.rs (content) These conflicts must be resolved before merging into main.	Resolve conflicts locally and push changes to this branch.
Linked Issues check	❓ Inconclusive	The linked issue #6326 requests refactoring topic name handling in ConsumerStateGetter for improved clarity, which is partially addressed in the broker_runtime.rs changes, but the PR includes substantial out-of-scope refactoring to NamespaceUtil and API signature changes that are not mentioned in the linked issue.	Clarify whether the namespace utility refactoring and API signature changes in NamespaceUtil were intended to address issue #6326, or whether they should be split into separate PRs with their own linked issues.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor-6326

⚔️ Resolve merge conflicts (beta)

• Auto-commit resolved conflicts to branch refactor-6326
• Create stacked PR with resolved conflicts
• Post resolved changes as copyable diffs in a comment

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

rocketmq-remoting/src/protocol/namespace_util.rs (1)

64-97: Clean refactoring of wrap_namespace to generic CheetahString-based API.

The conversion logic is correct, lifetimes are sound, and the early-return paths properly return the already-converted CheetahString. One small optimization opportunity on Line 94: &NamespaceUtil::NAMESPACE_SEPARATOR.to_string() allocates a String on every call just to represent the % character. Consider using a &str constant instead.

♻️ Avoid per-call allocation for the namespace separator

Introduce a &str constant and use it in the concatenation:

     pub const NAMESPACE_SEPARATOR: char = '%';
+    pub const NAMESPACE_SEPARATOR_STR: &'static str = "%";

Then in wrap_namespace:

-                + &NamespaceUtil::NAMESPACE_SEPARATOR.to_string()
+                + NamespaceUtil::NAMESPACE_SEPARATOR_STR

This avoids a heap allocation per call. The same pattern could be applied wherever NAMESPACE_SEPARATOR.to_string() is used (e.g., in is_already_with_namespace at Line 106 and without_namespace_with_namespace at Line 57).

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 73.91304% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.63%. Comparing base (870da38) to head (cae233a).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
rocketmq-client/src/base/client_config.rs	45.45%	6 Missing ⚠️
...producer/producer_impl/default_mq_producer_impl.rs	0.00%	3 Missing ⚠️
rocketmq-broker/src/broker_runtime.rs	0.00%	2 Missing ⚠️
rocketmq-remoting/src/protocol/namespace_util.rs	96.55%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6327      +/-   ##
==========================================
+ Coverage   42.62%   42.63%   +0.01%     
==========================================
  Files         912      912              
  Lines      128038   128056      +18     
==========================================
+ Hits        54570    54593      +23     
+ Misses      73468    73463       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[rocketmq-rust-bot]

LGTM - All CI checks passed ✅