RockteMQ-AI commented on code in PR #2134:
URL:
https://github.com/apache/rocketmq-dashboard/pull/2134#discussion_r3776364509
##########
server/src/test/java/com/rocketmq/studio/ops/audit/AuditServiceTest.java:
##########
@@ -107,6 +109,35 @@ void queryLogsShouldReturnEmptyWhenPageExceedsTotal() {
assertThat(result.getTotal()).isEqualTo(1);
}
+ @Test
Review Comment:
Corrupted test annotations. Lines use
`@apache_rocketmq/auth/src/test/java/...` instead of `@Test`, which will cause
compilation/test failures for the new audit tests.
##########
.github/workflows/ci.yml:
##########
@@ -0,0 +1,54 @@
+name: CI
+
+# 检测前端与后端是否都能正确编译
+on:
+ push:
+ branches:
+ - rocketmq-studio
+ pull_request:
+ branches:
+ - rocketmq-studio
+
+jobs:
+ backend-build:
+ name: Backend Build (Java 21)
+ runs-on: ubuntu-latest
+ defaults:
+ run:
+ working-directory: server
+ steps:
+ - name: Checkout
Review Comment:
Malformed action references: `uses: actions/checkout
@/tmp/repos/apache_rocketmq-clients/...` and setup-java/setup-node lines are
corrupted with local file paths instead of version tags like `@v4`. This will
break CI entirely.
##########
server/src/test/java/com/rocketmq/studio/ops/audit/AuditServiceTest.java:
##########
@@ -107,6 +109,35 @@ void queryLogsShouldReturnEmptyWhenPageExceedsTotal() {
assertThat(result.getTotal()).isEqualTo(1);
}
+ @Test
+ void queryLogsShouldRejectNonPositivePage() {
+ assertThatThrownBy(() -> auditService.queryLogs(0, 10, null, null,
null, null, null))
+ .isInstanceOf(BusinessException.class)
+ .hasMessage("page must be greater than 0")
+ .satisfies(ex -> assertThat(((BusinessException)
ex).getCode()).isEqualTo(400));
+ }
+
+ @Test
+ void queryLogsShouldRejectNonPositivePageSize() {
+ assertThatThrownBy(() -> auditService.queryLogs(1, 0, null, null,
null, null, null))
Review Comment:
Same corrupted annotation issue on additional new test methods; all five new
test methods need `@Test` restored.
##########
server/Dockerfile:
##########
@@ -6,6 +6,7 @@ WORKDIR /app
COPY pom.xml .
RUN mvn dependency:go-offline
COPY src ./src
+COPY style ./style
Review Comment:
Adding `COPY style ./style` is a correct and necessary fix for checkstyle
during Docker builds.
##########
server/src/main/java/com/rocketmq/studio/ops/audit/AuditService.java:
##########
@@ -38,6 +39,7 @@ public class AuditService {
public PageResult<AuditRecordVO> queryLogs(int page, int pageSize, String
search,
String operationType, String
startDate,
String endDate, String result) {
+ validatePagination(page, pageSize);
Review Comment:
Pagination parameter validation is a positive hardening change, but consider
whether `BusinessException(400, ...)` maps correctly to an HTTP 400 response in
the controller layer.
##########
server/src/main/java/com/rocketmq/studio/ops/audit/AuditService.java:
##########
@@ -47,20 +49,33 @@ public PageResult<AuditRecordVO> queryLogs(int page, int
pageSize, String search
List<AuditRecordVO> allRecords = auditRepository.findAll(search,
operationType, start, end, result);
long total = allRecords.size();
- int fromIndex = Math.min((page - 1) * pageSize, allRecords.size());
- int toIndex = Math.min(fromIndex + pageSize, allRecords.size());
+ long offset = (long) (page - 1) * pageSize;
Review Comment:
Offset overflow fix to `long` before casting back to `int` is correct and
addresses a real bug when `page * pageSize` exceeds `Integer.MAX_VALUE`.
##########
.claude/skills/pr-review/SKILL.md:
##########
@@ -0,0 +1,265 @@
+---
Review Comment:
Addition of a Claude-specific skill file is unrelated to the PR title 'hand
off home prompts to AI chat'. Unless this is intentional, it should be in a
separate PR or clearly documented.
##########
.github/workflows/ci.yml:
##########
@@ -0,0 +1,54 @@
+name: CI
Review Comment:
New CI workflow is a good addition, but it is bundled into a PR titled 'hand
off home prompts to AI chat' with no explanation, suggesting accidental
inclusion or poor scope separation.
--
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]
