iloveeyjafjalla opened a new pull request, #11424:
URL: https://github.com/apache/rocketmq/pull/11424
### Which Issue(s) This PR Fixes
- Fixes #11423
### Brief Description
`LocalTopicRouteService.getCurrentMessageQueueView` looked the topic up in
the local broker's topic config table and passed the result straight into the
route-data builder, so a topic that is not in the table produced a
`NullPointerException` while building the queue data:
```java
TopicConfig topicConfig =
this.brokerController.getTopicConfigManager().getTopicConfigTable().get(topic);
return new MessageQueueView(topic, toTopicRouteData(topicConfig), null);
// topicConfig may be null -> NPE at topicConfig.getPerm()
```
The cluster implementation of the same method already answers with a client
error — `getAllMessageQueueView` →
`TopicRouteService.getCacheMessageQueueWrapper` throws
`MQClientException(ResponseCode.TOPIC_NOT_EXIST, "No topic route info in name
server for the topic: …")`, and `ClusterTopicRouteServiceTest` pins that
behaviour. This change mirrors it for LOCAL mode:
```java
if (topicConfig == null) {
throw new MQClientException(ResponseCode.TOPIC_NOT_EXIST,
"No topic route info in local broker for the topic: " + topic);
}
```
so the callers can produce their designed responses instead of an internal
error — `ConsumerProcessor.popMessage` reaches `FORBIDDEN, "no readable queue"`
handling and `ProducerProcessor.sendMessage` (`FORBIDDEN, "no writable queue"`)
for topics that exist, while a genuinely missing topic is reported as
`TOPIC_NOT_EXIST`.
### How Did You Test This Change?
New test
`LocalTopicRouteServiceTest#testGetCurrentMessageQueueViewForUnknownTopic`:
calls `getCurrentMessageQueueView` for a topic that is not in the broker's
topic config table and asserts a `MQClientException` with
`ResponseCode.TOPIC_NOT_EXIST` (the existing `testGetCurrentMessageQueueView`
covers the topic-present case).
Results on Temurin JDK 8 (the JDK CI uses), base `a7edeecfb` plus this patch:
- before the fix: `java.lang.AssertionError` — the call throws
`NullPointerException` instead of `MQClientException`
- after the fix: `LocalTopicRouteServiceTest` — `Tests run: 3, Failures: 0,
Errors: 0`
- `mvn -B -ntp -pl proxy clean package -Dtest='!ProxyChannelTest'`: `BUILD
SUCCESS`, 453 tests / 0 failures / 4 skipped, `You have 0 Checkstyle
violations.`, `BugInstance size is 0`
Disclosure: `ProxyChannelTest#testWriteAndFlush` is excluded from that run
because it fails on the unmodified base as well (documented in earlier local
verification); `skipAfterFailureCount=1` is configured for this module, so
leaving it in would hide the rest of the suite.
This change was prepared with AI assistance (analysis, patch and test
drafting); the diff and every result above were reproduced locally by running
the module's tests against the base commit and the patched tree.
--
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]
