Copilot commented on code in PR #847:
URL:
https://github.com/apache/rocketmq-dashboard/pull/847#discussion_r3702980685
##########
server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertRuleController.java:
##########
@@ -59,4 +60,11 @@ public Result<Void> deleteRule(@Valid @RequestBody
DeleteAlertRuleDTO request) {
alertService.deleteRule(request.getId());
return Result.ok();
}
+
+ private AlertRuleVO requireAlertRule(AlertRuleVO rule) {
+ if (rule == null) {
+ throw new BusinessException(400, "Alert rule request is required");
+ }
+ return rule;
+ }
Review Comment:
The error code/message for a null alert rule request is duplicated across
controller and service. To avoid drift (e.g., message changes in one place but
not the other), centralize these into a shared constant (e.g., in
`AlertService`, a dedicated `AlertErrors` constants class, or an enum) and
reference it from both layers.
##########
server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertService.java:
##########
@@ -67,14 +67,20 @@ public String exportPrometheusRulesYaml() {
public AlertRuleVO createRule(AlertRuleVO rule) {
+ if (rule == null) {
+ throw new BusinessException(400, "Alert rule request is required");
+ }
Review Comment:
`400` is a magic number here. Consider using a named constant (project-wide
or local) or `HttpStatus.BAD_REQUEST.value()` to make intent clearer and reduce
the chance of inconsistent status codes across the codebase.
--
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]