[ISSUE #5333]✨Improve replicas_manager accessor to return Option<&T> in BrokerRuntimeInner

[WaterWhisperer]

Which Issue(s) This PR Fixes(Closes)

Fixes #5333

Brief Description

How Did You Test This Change?

Summary by CodeRabbit

• Refactor
  • Improved broker runtime API by refactoring how manager references are exposed to internal callers, resulting in a more ergonomic interface that better aligns with Rust best practices.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The public replicas_manager() accessor method in BrokerRuntimeInner is refactored to return Option<&ReplicasManager> instead of &Option<ReplicasManager>, with the implementation updated from &self.replicas_manager to self.replicas_manager.as_ref().

Changes

Cohort / File(s)	Summary
Accessor Return Type Refactoring rocketmq-broker/src/broker_runtime.rs	Method signature updated: replicas_manager() now returns Option<&ReplicasManager> instead of &Option<ReplicasManager>. Implementation adjusted to use .as_ref() for idiomatic option handling.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

• #5368: Converts additional accessor return types on BrokerRuntimeInner from references to Option<T> to Option<&T> pattern for other fields.
• #5351: Applies the same accessor refactoring pattern to the broker_stats method in BrokerRuntimeInner.

Suggested labels

enhancement✨, AI review first, Difficulty level/Easy, ready to review

Suggested reviewers

• SpaceXCN
• rocketmq-rust-bot
• mxsm

Poem

🐰 The accessor hops with grace,
Option references in their place,
No more &Option in the way,
Idiomatic Rust saves the day!
✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: improving the replicas_manager accessor to return Option<&T> instead of &Option.
Linked Issues check	✅ Passed	The code changes fully implement the requested enhancement from issue #5333: the replicas_manager accessor signature was changed from &Option to Option<&ReplicasManager>.
Out of Scope Changes check	✅ Passed	All changes are directly related to issue #5333; only the replicas_manager accessor signature and implementation were modified, with no extraneous modifications.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa7ed40 and cadb874.

📒 Files selected for processing (1)

• rocketmq-broker/src/broker_runtime.rs

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-05-10T06:20:00.401Z

Learnt from: 578223592
Repo: mxsm/rocketmq-rust PR: 3240
File: rocketmq-namesrv/src/kvconfig/kvconfig_mananger.rs:69-71
Timestamp: 2025-05-10T06:20:00.401Z
Learning: In RocketMQ Rust, while dashmap is concurrent-safe, returning a direct reference to a DashMap can break encapsulation by allowing external code to modify the map without triggering persistence methods like `persist()`. Consider returning a read-only view, iterator methods, or a snapshot instead.

Applied to files:

• rocketmq-broker/src/broker_runtime.rs

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Build & Test (windows-latest)
• GitHub Check: Code Coverage
• GitHub Check: auto-approve

🔇 Additional comments (1)

rocketmq-broker/src/broker_runtime.rs (1)

1632-1633: Good ergonomic improvement with correct implementation.

The refactoring from pub fn replicas_manager(&self) -> &Option<ReplicasManager> to pub fn replicas_manager(&self) -> Option<&ReplicasManager> is idiomatic Rust. The implementation using self.replicas_manager.as_ref() is correct. The existing call site in broker_epoch_cache_handler.rs:46 is compatible with the new signature and follows the same pattern.

---

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.}

[rocketmq-rust-bot]

🔊@WaterWhisperer 🚀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💥.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.26%. Comparing base (fa7ed40) to head (cadb874).
⚠️ Report is 9 commits behind head on main.

Files with missing lines	Patch %	Lines
rocketmq-broker/src/broker_runtime.rs	0.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5398   +/-   ##
=======================================
  Coverage   37.26%   37.26%           
=======================================
  Files         803      803           
  Lines      108756   108756           
=======================================
  Hits        40523    40523           
  Misses      68233    68233           

☔ 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 ✅

[mxsm]

LGTM