RockteMQ-AI commented on code in PR #2528:
URL:
https://github.com/apache/rocketmq-dashboard/pull/2528#discussion_r3831978021
##########
server/src/main/java/org/apache/rocketmq/studio/instance/group/ConsumerGroupController.java:
##########
@@ -66,6 +66,19 @@ public Result<ConsumerGroupVO> getConsumerGroup(
return Result.ok(metadataService.getConsumerGroup(instanceId, name));
}
+ @GetMapping("/{name}/settings")
+ public Result<ConsumerGroupSettingsVO>
getConsumerGroupSettings(@PathVariable String name,
+
@RequestParam String instanceId) {
+ return Result.ok(metadataService.getConsumerGroupSettings(instanceId,
name));
+ }
+
+ @PostMapping("/settings")
Review Comment:
**[Info]** Minor RESTful inconsistency: the GET endpoint uses
`/{name}/settings` (path variable) while the POST uses `/settings` (name in
request body). Consider aligning to `/{name}/settings` for the POST as well,
which is more RESTful and makes the URL self-describing for API consumers.
##########
server/src/main/java/org/apache/rocketmq/studio/instance/group/UpdateConsumerGroupSettingsDTO.java:
##########
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file except in compliance with the License.
+ */
+package org.apache.rocketmq.studio.instance.group;
+
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.Positive;
+import lombok.Data;
+
+@Data
+public class UpdateConsumerGroupSettingsDTO {
+ @NotBlank(message = "instanceId is required")
+ private String instanceId;
+ @NotBlank(message = "name is required")
+ private String name;
+ @NotNull(message = "retryQueueNums is required")
Review Comment:
**[Warning]** Missing upper-bound validation. The frontend caps
`retryQueueNums` and `retryMaxTimes` at 128, but the DTO only has `@Positive`
(ensures > 0). A direct API call could send arbitrarily large values (e.g.,
`retryQueueNums=999999`) which would be pushed to broker config without
validation.
Consider adding `@Max(128)` (or a reasonable upper bound) to match the
frontend constraints:
```java
@NotNull @Positive @Max(128)
private Integer retryQueueNums;
```
--
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]
