yyqdbngt opened a new pull request, #5345:
URL: https://github.com/apache/rocketmq-dashboard/pull/5345
### Which Issue(s) This PR Fixes
- Fixes #5114
### Brief Description
`AclUpdateToolHandler.parseId`
(`server/src/main/java/org/apache/rocketmq/studio/ops/ai/tool/handler/acl/AclUpdateToolHandler.java:71-79`)
converted the `rmq.acl.update` identifier into a `Long` unconditionally and
answered every non-numeric value with `ACL_ID_INVALID` ("ACL id must be
numeric: %s"). A Tencent ACL rule is identified by its role name —
`TencentAclService` reads it from the rule principal — and
`AclService.updateRule` already routes a Tencent instance to that provider, so
the MCP handler refused a call the service path supports. The write path
therefore exposed fewer provider capabilities over MCP/`rmqctl` than over REST.
The fix keeps the blank-identifier guard and returns `null` for an
identifier that is not numeric, leaving the numeric conversion to the
provider-aware service path: Tencent updates the role named by the principal,
while `AclService.updateRule` still rejects a rule without a numeric id when
the target instance is Apache-backed. The plan/preview path is unchanged and
keeps using `AclService.getRule(id, instanceId)`, which already accepts a
Tencent role name.
Files:
-
`server/src/main/java/org/apache/rocketmq/studio/ops/ai/tool/handler/acl/AclUpdateToolHandler.java`
-
`server/src/test/java/org/apache/rocketmq/studio/ops/ai/tool/handler/acl/AclUpdateToolHandlerTest.java`
### How Did You Test This Change?
Red — the new case against the unmodified handler:
```
$ cd server && mvn -B -ntp test -Dtest=AclUpdateToolHandlerTest
[ERROR] Tests run: 3, Failures: 0, Errors: 1, Skipped: 0, Time elapsed:
2.455 s <<< FAILURE! -- in
org.apache.rocketmq.studio.ops.ai.tool.handler.acl.AclUpdateToolHandlerTest
[ERROR]
org.apache.rocketmq.studio.ops.ai.tool.handler.acl.AclUpdateToolHandlerTest.applyShouldPassATencentRoleNameIdentifierThroughToTheService
-- Time elapsed: 0.041 s <<< ERROR!
org.apache.rocketmq.studio.ops.ai.tool.core.ToolExecutionException: ACL id
must be numeric: role-orders
at
org.apache.rocketmq.studio.ops.ai.tool.core.ToolError.exception(ToolError.java:173)
at
org.apache.rocketmq.studio.ops.ai.tool.handler.acl.AclUpdateToolHandler.parseId(AclUpdateToolHandler.java:76)
at
org.apache.rocketmq.studio.ops.ai.tool.handler.acl.AclUpdateToolHandler.execute(AclUpdateToolHandler.java:66)
at
org.apache.rocketmq.studio.ops.ai.tool.handler.acl.AclUpdateToolHandlerTest.applyShouldPassATencentRoleNameIdentifierThroughToTheService(AclUpdateToolHandlerTest.java:85)
[ERROR] Tests run: 3, Failures: 0, Errors: 1, Skipped: 0
[INFO] BUILD FAILURE
```
Green — after the fix, same test file:
```
$ cd server && mvn -B -ntp test -Dtest=AclUpdateToolHandlerTest
[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.193
s -- in
org.apache.rocketmq.studio.ops.ai.tool.handler.acl.AclUpdateToolHandlerTest
[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```
The added cases are
`applyShouldPassATencentRoleNameIdentifierThroughToTheService` (the role name
reaches `AclService.updateRule` for the Tencent instance, no numeric id is
synthesised, the principal is carried) and
`applyShouldStillRejectAMissingIdentifier` (a blank id is still refused by the
handler, and the service is never touched), on top of the existing numeric-id
case. The whole ACL handler package was re-run with the fix in place:
```
$ cd server && mvn -B -ntp test
-Dtest=org.apache.rocketmq.studio.ops.ai.tool.handler.acl.*Test
[INFO] Tests run: 17, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```
Checkstyle runs at `validate` in the same command and reported no violations.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits
- [x] Tests added or updated for non-trivial changes
- [x] New UI text has both Chinese and English entries under `web/src/i18n/`
— not applicable: no UI text changed
- [x] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks) — no package boundary or dependency changed; the checkstyle gate ran at
`validate`
- [x] New source files carry the ASF license header — no new source file
- [x] Documentation touched where behaviour changed (README / `docs/` /
in-app help) — `ToolError.ACL_ID_INVALID` keeps its meaning (the identifier is
absent); the tool's declared contract is unchanged, so no `docs/` change was
needed
--
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]
