Copilot commented on code in PR #3646:
URL:
https://github.com/apache/rocketmq-dashboard/pull/3646#discussion_r3941770177
##########
server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertRuleController.java:
##########
@@ -66,6 +67,17 @@ public Result<PageResult<AlertRuleVO>> listRulesPage(
return Result.ok(alertService.listRules(AlertDomain.BUSINESS, search,
enabled, page, pageSize));
}
+ @GetMapping("/summary")
+ public Result<AlertRuleSummaryVO> summarizeRules(
+ HttpServletRequest request,
+ @RequestParam(required = false) String search,
+ @RequestParam(required = false) Boolean enabled) {
+ AlertDomain domain =
request.getRequestURI().endsWith("/api/alert-rules/summary")
+ ? AlertDomain.CLUSTER
+ : AlertDomain.BUSINESS;
+ return Result.ok(alertService.summarizeRules(domain, search, enabled,
LocalDateTime.now().minusDays(1)));
+ }
Review Comment:
`/summary` is only implemented on `AlertRuleController` (mapped to
`/api/alert-rules` + `/api/business-alert-rules`), but the frontend calls the
CLUSTER route at `/api/cluster-alert-rules/summary` (see `web/src/api/ops.ts`),
which currently has no handler. This will 404 for CLUSTER and silently fall
back to page-local counters, so the header still mixes scopes. Add the same
`/summary` endpoint to `ClusterAlertRuleController` (and ideally remove the
URI-suffix domain switch here once each controller is domain-specific).
##########
server/src/main/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertRepository.java:
##########
@@ -100,6 +100,48 @@ public PageResult<AlertRuleVO>
findRulesPage(AlertRuleQuery query) {
result.getTotal(), query.page(), query.pageSize());
}
+ @Override
+ public AlertRuleSummaryVO summarizeRules(AlertRuleQuery query) {
+ QueryWrapper<RmqAlertRule> conditions = ruleSummaryConditions(query);
+ List<Map<String, Object>> rows = ruleMapper.selectMaps(conditions);
+ Map<String, Object> row = rows.isEmpty() ? Map.of() : rows.get(0);
+ return AlertRuleSummaryVO.builder()
+ .total(asLong(row, "total_count"))
+ .enabled(asLong(row, "enabled_count"))
+ .triggeredSince(asLong(row, "triggered_count"))
+ .build();
+ }
+
+ private QueryWrapper<RmqAlertRule> ruleSummaryConditions(AlertRuleQuery
query) {
+ QueryWrapper<RmqAlertRule> conditions = new
QueryWrapper<RmqAlertRule>()
+ .select(
+ "COUNT(*) AS total_count",
+ "COALESCE(SUM(CASE WHEN enabled = 1 THEN 1 ELSE 0
END), 0) AS enabled_count",
+ "COALESCE(SUM(CASE WHEN last_triggered IS NOT NULL "
+ + "AND last_triggered >= '" +
query.triggeredSince() + "' THEN 1 ELSE 0 END), 0) "
+ + "AS triggered_count")
+ .eq(query.enabled() != null, "enabled", query.enabled())
Review Comment:
`ruleSummaryConditions` concatenates `query.triggeredSince()` directly into
the SQL select expression. This can generate invalid SQL when `triggeredSince`
is null (producing `>= 'null'`), and it also bypasses SQL parameter
binding/escaping. At minimum, handle null and escape single quotes before
embedding the value.
--
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]
