RockteMQ-AI commented on code in PR #2555:
URL:
https://github.com/apache/rocketmq-dashboard/pull/2555#discussion_r3849608280
##########
web/src/services/topicService.ts:
##########
@@ -54,6 +56,21 @@ export async function listTopicsPage(
return metadataApi.listTopicsPage(params);
}
+export const listAllTopics = async (params: TopicQuery = {}): Promise<Topic[]>
=> {
+ const topics: Topic[] = [];
+ let page = 1;
+
+ while (true) {
+ const result = await listTopicsPage({ ...params, page, pageSize:
EXPORT_PAGE_SIZE });
+ topics.push(...result.items);
+ const total = result.total ?? topics.length;
+ if (result.items.length === 0 || topics.length >= total) break;
+ page += 1;
Review Comment:
**[Info]** `listAllConsumerGroups` applies `normalizeConsumerGroup()` to
results, but `listAllTopics` returns raw topics without any normalization. If
topics don't need normalization this is fine, but worth confirming for
consistency.
##########
web/src/services/consumerService.ts:
##########
@@ -83,6 +84,27 @@ export async function listConsumerGroupPage(
return metadataApi.listConsumerGroupPage(params);
}
+export async function listAllConsumerGroups(
+ params: ConsumerGroupQuery = {},
+): Promise<ConsumerGroup[]> {
+ const groups: ConsumerGroup[] = [];
+ let page = 1;
+
Review Comment:
**[Warning]** The `while (true)` loop has no upper-bound safety guard. If
the API returns inconsistent `total` (e.g., always growing), this could loop
indefinitely. Consider adding a max-page cap:
```typescript
const MAX_PAGES = 100; // safety net
while (page <= MAX_PAGES) {
...
}
```
Same applies to `listAllTopics` in `topicService.ts:62`.
##########
web/src/pages/instance/topic.tsx:
##########
@@ -555,6 +567,28 @@ const TopicPage = () => {
}
};
+ const handleExport = () => {
+ setExporting(true);
+
+ void listAllTopics({
+ instanceId: selectedInstanceId || undefined,
+ type: typeFilter || undefined,
+ search: searchText.trim() || undefined,
+ })
Review Comment:
**[Info]** `listAllTopics` is called with `search`/`type`/`instanceId`
filters, then `visibleTopics()` re-applies the same filters on the result. The
second filtering pass is redundant since the API already filtered. Not a bug,
just slightly wasteful — could simplify to `const exportTopics = allTopics;` or
skip passing filters to `listAllTopics` and rely solely on `visibleTopics`.
--
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]
