Copilot commented on code in PR #834:
URL:
https://github.com/apache/rocketmq-dashboard/pull/834#discussion_r3702736422
##########
server/src/main/java/org/apache/rocketmq/studio/ops/ai/AiController.java:
##########
@@ -41,12 +42,18 @@ public class AiController {
private final AiService aiService;
@PostMapping(value = "/chat", produces = MediaType.TEXT_EVENT_STREAM_VALUE)
- public SseEmitter chat(@RequestBody ChatDTO request) {
+ public SseEmitter chat(@RequestBody(required = false) ChatDTO request) {
+ if (request == null) {
+ throw new BusinessException(400, "chat request is required");
+ }
return aiService.chat(request);
}
@PostMapping("/execute")
- public Result<AiExecuteResultVO> execute(@RequestBody AiCommandDTO
command) {
+ public Result<AiExecuteResultVO> execute(@RequestBody(required = false)
AiCommandDTO command) {
+ if (command == null) {
+ throw new BusinessException(400, "AI command request is required");
+ }
return Result.ok(aiService.execute(command));
}
Review Comment:
The null-request validation is duplicated in both controller and service.
This increases the chance of divergence (message/code changes in one place but
not the other) and adds extra maintenance. Consider keeping
`@RequestBody(required = false)` in the controller (so missing bodies bind to
null), but delegating the null check to `AiService` only (remove the controller
`if (request/command == null)` blocks), or extracting a shared validator/helper
to keep behavior consistent.
##########
server/src/main/java/org/apache/rocketmq/studio/ops/ai/AiService.java:
##########
@@ -34,12 +35,18 @@ public class AiService {
public SseEmitter chat(ChatDTO request) {
+ if (request == null) {
+ throw new BusinessException(400, "chat request is required");
+ }
log.info("Chat request received: mode={}, conversationId={}",
request.getMode(), request.getConversationId());
return llmGateway.chat(request);
}
public AiExecuteResultVO execute(AiCommandDTO command) {
+ if (command == null) {
+ throw new BusinessException(400, "AI command request is required");
+ }
Review Comment:
Using the literal `400` as a magic number makes intent less explicit and can
lead to inconsistent usage across the codebase. Prefer a named constant (e.g.,
`HttpStatus.BAD_REQUEST.value()`) or a shared app constant for the error code
to improve readability and consistency.
##########
server/src/main/java/org/apache/rocketmq/studio/ops/ai/AiService.java:
##########
@@ -34,12 +35,18 @@ public class AiService {
public SseEmitter chat(ChatDTO request) {
+ if (request == null) {
+ throw new BusinessException(400, "chat request is required");
+ }
Review Comment:
Using the literal `400` as a magic number makes intent less explicit and can
lead to inconsistent usage across the codebase. Prefer a named constant (e.g.,
`HttpStatus.BAD_REQUEST.value()`) or a shared app constant for the error code
to improve readability and consistency.
--
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]
