Copilot commented on code in PR #841:
URL:
https://github.com/apache/rocketmq-dashboard/pull/841#discussion_r3702900345
##########
server/src/test/java/org/apache/rocketmq/studio/auth/AuthControllerTest.java:
##########
@@ -144,6 +145,17 @@ void loginShouldReturnAdminUserWhenAdminLogsIn() throws
Exception {
.andExpect(jsonPath("$.data.user.admin").value(true));
}
+ @Test
+ void loginShouldRejectMissingRequestBody() throws Exception {
+ mockMvc.perform(post("/api/auth/login")
+ .contentType(MediaType.APPLICATION_JSON))
+ .andExpect(status().isBadRequest())
+ .andExpect(jsonPath("$.code").value(400))
+ .andExpect(jsonPath("$.message").value("Login request is
required"));
+
+ verify(authService, never()).login(any(LoginDTO.class));
Review Comment:
`any(LoginDTO.class)` does not match `null` in Mockito, so this verification
can still pass even if `authService.login(null)` was called. Use a matcher that
covers null (e.g., `Mockito.<LoginDTO>any()`), or verify no interactions (e.g.,
`verifyNoInteractions(authService)` / `verify(authService,
never()).login(isNull())` depending on intent).
##########
server/src/main/java/org/apache/rocketmq/studio/auth/AuthController.java:
##########
@@ -50,7 +51,10 @@ public ResponseEntity<Result<AuthStatusVO>> status(
}
@PostMapping("/login")
- public Result<LoginVO> login(@RequestBody LoginDTO request) {
+ public Result<LoginVO> login(@RequestBody(required = false) LoginDTO
request) {
+ if (request == null) {
+ throw new BusinessException(400, "Login request is required");
+ }
return Result.ok(authService.login(request));
Review Comment:
The controller null-guard duplicates the service null-guard and hard-codes
the same message/code in two places, which is easy to let drift over time.
Consider removing this controller check and letting `AuthService.login(null)`
raise the BusinessException (and update the controller test accordingly), or
centralize the error construction/message in a shared constant/helper used by
both layers.
##########
server/src/main/java/org/apache/rocketmq/studio/auth/AuthService.java:
##########
@@ -50,6 +50,10 @@ public AuthService(AuthProperties authProperties) {
}
public LoginVO login(LoginDTO request) {
+ if (request == null) {
+ throw new BusinessException(400, "Login request is required");
+ }
Review Comment:
Avoid hard-coded HTTP status numbers in exceptions. Prefer a named constant
(e.g., `HttpStatus.BAD_REQUEST.value()`) or a project-standard status constant
to improve readability and reduce duplication (also applies to the same `400`
used in the controller).
--
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]
