yyqdbngt opened a new pull request, #4690:
URL: https://github.com/apache/rocketmq-dashboard/pull/4690
<!-- Base branch: `rocketmq-studio`, the RocketMQ Studio trunk. -->
### Brief Description
`AclRuleItem.from` and `AclUserItem.from` turned a missing numeric id into
`null`, and both records
are annotated `@JsonInclude(JsonInclude.Include.NON_NULL)`, so the `id`
property disappeared from
the tool result entirely.
That is not a corner case: Tencent Cloud is the provider where the numeric
id is *always* absent.
`TencentAclService.toRule` and `TencentAclService.toUser` never set it —
"Tencent roles have no
database row; the role name lives in username, id stays null" — and
`AclService.getRule`/`getUser`
accept that role name as the identifier for exactly that reason:
```java
.filter(rule -> id.equals(rule.getPrincipal())
|| rule.getId() != null && id.equals(rule.getId().toString()))
```
The published output schemas require a non-null `id` for `rmq.acl.list`,
`rmq.acl.get`,
`rmq.user.list` and `rmq.user.get` (`required: [id, ...]`, `id: {type:
string}`), so on a cloud
Instance all four tools failed output validation inside the filter chain and
surfaced as an
internal error (HTTP 500 / `INTERNAL_ERROR`) instead of returning the ACL
data.
The projections now fall back to the identifier the ACL service itself
accepts for a rule or user
without a numeric id — the principal, respectively the username — so the
payload satisfies its
schema and `rmq.user.get`/`rmq.user.delete`/`rmq.acl.get`/`rmq.acl.update`
keep round-tripping the
value they are handed. Apache/MySQL-backed Instances are unchanged: there
the numeric id exists and
is still projected as before (covered by the third test case).
### How Did You Test This Change?
New test `CloudAclIdentifierProjectionTest` projects the cloud-shaped
`AclRuleVO`/`AclUserVO`
(numeric id null, role name in the principal/username) and validates the
result through the same
`ToolSchemaValidator` the runtime filter chain uses, so the failure and the
fix are observed on the
published contract rather than on a field assertion alone.
Before the fix (red):
```
$ cd server && mvn -B -ntp test -Dtest=CloudAclIdentifierProjectionTest
[ERROR] Tests run: 3, Failures: 0, Errors: 2, Skipped: 0
[ERROR]
CloudAclIdentifierProjectionTest.aclRuleWithoutNumericIdKeepsTheSchemasRequiredIdTest:62
? IllegalState Tool output validation failed for rmq.acl.get: [:
required property 'id' not found]
[ERROR]
CloudAclIdentifierProjectionTest.aclUserWithoutNumericIdKeepsTheSchemasRequiredIdTest:78
? IllegalState Tool output validation failed for rmq.user.get: [:
required property 'id' not found]
```
After the fix (green) — the whole tool package, so the neighbouring handler
and contract tests are
covered as well:
```
$ cd server && mvn -B -ntp test
-Dtest='org.apache.rocketmq.studio.ops.ai.tool.**.*Test'
[INFO] Tests run: 156, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```
`mvn test` runs checkstyle in the `validate` phase: `You have 0 Checkstyle
violations.`
Note on the full suite: on a clean `rocketmq-studio` checkout `mvn -B -ntp
test` already reports
`Tests run: 3051, Failures: 6, Errors: 25, Skipped: 4`. The 11 red classes
are the MySQL 8 backed
Spring integration tests (`AuthServiceBootstrapIntegrationTest`,
`AuthServiceConcurrencyIntegrationTest`,
`AuthServiceSessionOverviewIntegrationTest`,
`HealthProbeIntegrationTest`, `QueryHistoryServiceIntegrationTest`,
`NativeAlertEvaluationTransactionTest`,
`NotificationOutboxMapperIntegrationTest`,
`RmqAlertStateMapperIntegrationTest`, `StudioApplicationTest`) plus the
external-CLI ones
(`CliAgentProviderTest`, `ClaudeCodeAgentProviderTest`). None of them are
touched by this change.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`fix:`)
- [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/`
(no UI text in this change)
- [x] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks)
- [x] New source files carry the ASF license header
- [x] Documentation touched where behaviour changed (no user-visible
contract change: the schemas already required a non-null `id`, this makes the
projections honour it)
--
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]
