Copilot commented on code in PR #861:
URL:
https://github.com/apache/rocketmq-dashboard/pull/861#discussion_r3704394218
##########
server/src/main/java/org/apache/rocketmq/studio/cluster/broker/ClusterController.java:
##########
@@ -55,7 +56,8 @@ public Result<ClusterVO> getCluster(@PathVariable String id) {
}
@PostMapping("/config/update")
- public Result<ClusterVO> updateClusterConfig(@Valid @RequestBody
UpdateConfigDTO command) {
+ public Result<ClusterVO> updateClusterConfig(@Valid @RequestBody(required
= false) UpdateConfigDTO command) {
+ requireUpdateConfigCommand(command);
return Result.ok(clusterService.updateClusterConfig(command));
}
Review Comment:
Setting `@RequestBody(required = false)` changes behavior beyond JSON
`null`: requests with a missing/empty body will now reach the controller and be
mapped to `null`, producing your `BusinessException` response instead of
Spring’s default `HttpMessageNotReadableException` handling. If the intent is
strictly to reject JSON `null` while keeping prior behavior for missing bodies,
consider keeping `required = true` and enforcing non-null via Bean Validation
on the parameter (e.g., `@NotNull`) with method validation enabled, or handling
only the JSON-`null` case via message conversion/exception handling.
##########
server/src/main/java/org/apache/rocketmq/studio/cluster/broker/ClusterController.java:
##########
@@ -68,4 +70,10 @@ public Result<Map<String, Object>>
restartBroker(@PathVariable String clusterId,
"message", "Broker restart initiated for " + name
));
}
+
+ private void requireUpdateConfigCommand(UpdateConfigDTO command) {
+ if (command == null) {
+ throw new BusinessException(400, "Cluster config update request is
required");
+ }
+ }
Review Comment:
Avoid the magic number `400` here; it’s clearer and less error-prone to
reference a shared constant (e.g., `HttpStatus.BAD_REQUEST.value()`) or a
project-level error code enum/constant if one exists. This also makes refactors
and consistency checks easier across controllers.
--
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]
