Copilot commented on code in PR #851:
URL:
https://github.com/apache/rocketmq-dashboard/pull/851#discussion_r3703890170
##########
server/src/main/java/org/apache/rocketmq/studio/cluster/nameserver/NameServerController.java:
##########
@@ -36,31 +37,45 @@ public class NameServerController {
private final ClusterService clusterService;
@PostMapping("/create")
- public Result<NameServerVO> createNameServer(@Valid @RequestBody
CreateNameServerDTO command) {
+ public Result<NameServerVO> createNameServer(@Valid @RequestBody(required
= false) CreateNameServerDTO command) {
+ requireCommand(command);
return Result.ok(clusterService.createNameServer(command));
}
@PostMapping("/update")
- public Result<Void> updateNameServer(@Valid @RequestBody
UpdateNameServerDTO command) {
+ public Result<Void> updateNameServer(@Valid @RequestBody(required = false)
UpdateNameServerDTO command) {
+ requireCommand(command);
clusterService.updateNameServer(command);
return Result.ok();
}
@PostMapping("/restart")
- public Result<Map<String, Boolean>> restartNameServer(@Valid @RequestBody
RestartNameServerDTO command) {
+ public Result<Map<String, Boolean>> restartNameServer(
+ @Valid @RequestBody(required = false) RestartNameServerDTO
command) {
+ requireCommand(command);
boolean success = clusterService.restartNameServer(command);
return Result.ok(Map.of("success", success));
}
@PostMapping("/upgrade")
- public Result<Map<String, Boolean>> upgradeNameServer(@Valid @RequestBody
UpgradeNameServerDTO command) {
+ public Result<Map<String, Boolean>> upgradeNameServer(
+ @Valid @RequestBody(required = false) UpgradeNameServerDTO
command) {
+ requireCommand(command);
boolean success = clusterService.upgradeNameServer(command);
return Result.ok(Map.of("success", success));
}
@PostMapping("/delete")
- public Result<Map<String, Boolean>> deleteNameServer(@Valid @RequestBody
DeleteNameServerDTO command) {
+ public Result<Map<String, Boolean>> deleteNameServer(
+ @Valid @RequestBody(required = false) DeleteNameServerDTO command)
{
+ requireCommand(command);
boolean success = clusterService.deleteNameServer(command);
return Result.ok(Map.of("success", success));
}
+
+ private void requireCommand(Object command) {
+ if (command == null) {
+ throw new BusinessException(400, "NameServer request is required");
+ }
+ }
Review Comment:
The HTTP code `400` and message string are hard-coded here and also
duplicated in `ClusterService.requireNameServerCommand`, which risks future
drift (e.g., one side changed but not the other). Consider extracting these
into shared constants (or a small shared helper/factory for
`BusinessException`) and using `HttpStatus.BAD_REQUEST.value()` (or an existing
project-wide constant) instead of the magic number.
##########
server/src/test/java/org/apache/rocketmq/studio/cluster/nameserver/NameServerControllerTest.java:
##########
@@ -71,6 +71,28 @@ void createNameServerShouldPassValidatedRequest() throws
Exception {
verify(clusterService).createNameServer(any(CreateNameServerDTO.class));
}
+ @Test
+ void nameServerWriteEndpointsShouldRejectNullRequestBody() throws
Exception {
+ String[] paths = {
+ "/api/nameservers/create",
+ "/api/nameservers/update",
+ "/api/nameservers/restart",
+ "/api/nameservers/upgrade",
+ "/api/nameservers/delete"
+ };
+
+ for (String path : paths) {
+ mockMvc.perform(post(path)
+ .contentType(MediaType.APPLICATION_JSON)
+ .content("null"))
+ .andExpect(status().isBadRequest())
+ .andExpect(jsonPath("$.code").value(400))
+ .andExpect(jsonPath("$.message").value("NameServer request
is required"));
+ }
Review Comment:
This test covers the explicit JSON literal `null`, but the controller
changes also set `@RequestBody(required = false)`, which affects the behavior
when the body is completely missing. Add an additional assertion per endpoint
for the 'no body sent' case (i.e., omit `.content(...)`) to ensure the new
controller guard consistently returns the expected 400 error in that scenario
too.
--
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]
