fix: guard NPE in TopicRouteWrapper when brokerName is absent from the route

[yyqdbngt]

Motivation

TopicRouteWrapper.getMasterAddr and getMasterAddrPrefer called brokerNameRouteData.get(brokerName).getBrokerAddrs() without a null check. When brokerName is not present in the current route snapshot (e.g. during broker failover, or a stale MessageQueue referencing a broker no longer in the route), get(brokerName) returns null and the chained getBrokerAddrs() throws NullPointerException.

This contradicts the contract the callers rely on. In MessageQueueSelector.buildRead/buildWrite and ClusterTopicRouteService, every call site checks the return value for null and skips the broker:

String brokerAddr = topicRoute.getMasterAddr(qd.getBrokerName());
if (brokerAddr == null) {
    continue;
}

So these methods are supposed to return null for a missing broker, but instead they throw. The fix guards the lookup and returns null, making the existing caller handling work as intended.

Verification

mvn -pl proxy -am compile passes on the build server.

Diff

1 file changed, +10 / -2.

[RockteMQ-AI]

Review by github-manager-bot

Summary

This PR adds null guards in TopicRouteWrapper.getMasterAddr() and getMasterAddrPrefer() to prevent NPE when brokerName is not present in the current route snapshot.

Findings

• [Info] TopicRouteWrapper.java:46-50 — The null guard for brokerNameRouteData.get(brokerName) is correct and aligns with the caller contract (callers already check for null return). This is a valid defensive fix for scenarios like broker failover or stale MessageQueue references.

• [Info] TopicRouteWrapper.java:53-57 — Same null guard applied to getMasterAddrPrefer(). Correct.

• [Warning] TopicRouteWrapper.java:58 — After the null guard, brokerData.getBrokerAddrs() could theoretically also return null (depending on BrokerData construction). Consider whether a secondary null check or a Collections.emptyMap() fallback is warranted here. This is a pre-existing concern, not introduced by this PR, but worth noting since you are already hardening this class.

• [Info] No unit test is included. A simple test case covering the brokerName absent scenario (verifying null is returned instead of NPE) would strengthen this fix and prevent regression.

Suggestions

1. Consider adding a test in a new TopicRouteWrapperTest class:

@Test
public void testGetMasterAddr_missingBroker_returnsNull() {
    TopicRouteData routeData = new TopicRouteData();
    routeData.setBrokerDatas(new ArrayList<>()); // empty
    TopicRouteWrapper wrapper = new TopicRouteWrapper(routeData, "testTopic");
    assertNull(wrapper.getMasterAddr("nonExistentBroker"));
    assertNull(wrapper.getMasterAddrPrefer("nonExistentBroker"));
}

Verdict

The fix is correct, minimal, and addresses a real NPE risk. LGTM with the suggestion to add test coverage.

---

Automated review by github-manager-bot