zjncs opened a new pull request, #4574:
URL: https://github.com/apache/rocketmq-dashboard/pull/4574

   ### Which Issue(s) This PR Fixes
   
   No issue — found by sweeping every catalog tool's `required` output fields 
against the nullability its provider paths actually produce (same class as the 
alert `metric` gap in #4571 / #4186).
   
   ### Brief Description
   
   `TencentAclService` deliberately returns rules and users **without a 
database id** — its own comments say "Tencent roles have no database row; the 
role name lives in username, id stays null" and "For Tencent roles the id is 
the role name" (`AclService#deleteUser`). `AclService` accordingly looks 
rules/users up by principal/username as well as by id.
   
   But the output schemas of `rmq.acl.get`, `rmq.acl.list`, `rmq.user.get`, 
`rmq.user.list` and the `rmq.user.create` result all declare `id` as 
**required**. `AclRuleItem` and `AclUserItem` serialize with 
`@JsonInclude(NON_NULL)`, so a Tencent rule/user is emitted without an `id` 
key, and `ToolValidationFilter` rejects the whole response with `required 
property 'id' not found`. For an instance bound to Tencent Cloud, every ACL and 
user tool call fails.
   
   Fix: declare `id` as `[string, 'null']` and drop it from the required lists 
— the same convention `acl.yaml` already uses for `gmtCreate` (also null on 
Tencent paths). The tools remain usable because the service already accepts the 
role name as the `id` argument of get/update/delete.
   
   ### How Did You Test This Change?
   
   `cd server && mvn -B -ntp test -Dtest='ToolOutputSchemaContractTest' 
-Dspotbugs.skip=true` (Java 21), after adding null-id Tencent-shaped samples 
for `rmq.acl.get`, `rmq.acl.list`, `rmq.user.get`, `rmq.user.list` and 
`rmq.user.create` to the sweep:
   
   - before this change: `Tool output validation failed for rmq.acl.get: [: 
required property 'id' not found]` (the list/get/user samples fail identically 
— the sweep reports the first)
   - after this change: `Tests run: 2, Failures: 0, Errors: 0`
   
   Full tool layer: `mvn -B -ntp test -Dtest='*Tool*Test' -Dspotbugs.skip=true` 
→ `Tests run: 127, Failures: 0, Errors: 0`.
   
   ### Checklist
   
   - [x] One coherent change; unrelated modifications are not bundled in
   - [x] Commit subject follows Conventional Commits (`feat:` / `fix:` / 
`refactor:` / `chore:` / `docs:` / `perf:`)
   - [x] Tests added or updated for non-trivial changes, test methods named 
`...Test`
   - [x] New UI text has both Chinese and English entries under `web/src/i18n/` 
(n/a — no UI change)
   - [x] Architecture constraints stay green (`mvn test` runs the ArchUnit 
checks)
   - [x] New source files carry the ASF license header (n/a — no new files)
   - [x] Documentation touched where behaviour changed (n/a — schema now 
matches the documented Tencent provider behaviour)
   


-- 
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]

Reply via email to