unbridled-41 opened a new pull request, #4891:
URL: https://github.com/apache/rocketmq-dashboard/pull/4891
## Problem
The ACL rule table's **"ACL 1.0 / ACL 2.0"** selector does nothing in
production: picking a version leaves every rule in the table and the pagination
total unchanged. The control only appears to work in demo/mock mode.
## Evidence
The value is sent and the query predicate exists — only the wiring between
them is missing.
- The console sends it: `web/src/pages/instance/acl.tsx:196` → `aclVersion:
ruleVersionFilter === 'all' ? undefined : ruleVersionFilter`, forwarded by
`web/src/api/acl.ts:63` (`client.get('/acl/rules', { params })`).
- The controller never bound it (base `1ef5d860`,
`AclController.java:58-69`): `listRules(@RequestParam principal, resource,
scope, decision, instanceId, page, pageSize)` — Spring silently drops an
unknown query parameter, so `?aclVersion=1.0` was discarded at the boundary.
- The service hardcoded it: `AclService.java:98` passed `null` in the
repository's `acl_version` slot — `aclRepository.findRulePage(principal,
resource, scope, decision, null, …)`.
- The predicate was already there and unused:
`MybatisPlusAclRepository.java:401` — `.eq(StringUtils.hasText(aclVersion),
"acl_version", aclVersion)`.
- The demo path filters client-side
(`web/src/services/aclService.ts:60-62`), which is why the selector looks
functional in mock mode. The API test that sends the parameter even describes
it as a "controller-supported ACL rule filter" (`web/src/api/acl.test.ts:42`) —
the controller did not support it.
Reproduction against the unfixed source, with the real `AclService` between
the controller and a mocked repository — `GET /api/acl/rules?aclVersion=1.0`:
```
Argument(s) are different! Wanted: findRulePage(null, null, null, null,
"1.0", 1, 20)
Actual invocations have different arguments at position [4]:
findRulePage(null, null, null, null, null, 1, 20)
-> at AclService.listRules(AclService.java:98)
```
Concrete user-visible effect: with `orders-read` (`acl_version` 2.0) and
`payout-role` (1.0), selecting **ACL 1.0** answers `GET
/api/acl/rules?aclVersion=1.0&page=1&pageSize=20` with **both** rules and
`total: 2`; the expected answer is one row and `total: 1`.
## Root cause
A filter was added to the client (and to the mock), but the parameter was
never threaded through the controller and service to the repository predicate
that had been written for it.
## Fix
`aclVersion` is now a documented filter of `GET /api/acl/rules`, and it
travels with the other filters: the controller binds it, `AclService.listRules`
forwards it to the repository (no more hardcoded `null`), and the role-backed
projection (Tencent), which has no `acl_version` column, filters it in memory
beside the resource/scope/decision filters it already applied — its rules are
projected with `ACL_VERSION = "1.0"`, i.e. the same vocabulary the selector
offers.
The service takes one canonical signature carrying every filter rather than
an overload, so a filter cannot be silently dropped by a future caller the way
this one was.
Scope note: the AI tool contract (`rmq.acl.list`) has no version input, so
that path passes no version filter and is unchanged; adding it would be a
catalog change of its own.
## Tests
- `server/src/test/java/.../instance/acl/AclRuleVersionFilterWebTest.java`
(new): a `@WebMvcTest(AclController.class)` slice with the **real**
`AclService` and a mocked `AclRepository`, so it fails unless the request
parameter survives both layers. Two cases: with `aclVersion=1.0` the repository
must be queried with `"1.0"`, and without it the predicate must stay open
(`null`).
- `AclServiceTest.listRulesShouldFilterTencentRulesByAclVersion` (new): the
in-memory filter for the role-backed projection.
- Pre-fix failure of the new web test is quoted above (test kept, main
sources reverted).
- `mvn -o test
-Dtest='org.apache.rocketmq.studio.instance.acl.*Test,org.apache.rocketmq.studio.ops.ai.tool.handler.acl.*Test,org.apache.rocketmq.studio.provider.tencent.TencentAclServiceTest'`
→ **Tests run: 210, Failures: 0, Errors: 0, BUILD SUCCESS**.
- Full `mvn -o test` → **Tests run: 3153, Failures: 0, Errors: 17**; all 17
are `Failed to load ApplicationContext` in MySQL-backed integration tests
(`AuthService*IntegrationTest`, `HealthProbeIntegrationTest`,
`NotificationOutboxMapperIntegrationTest`,
`RmqAlertStateMapperIntegrationTest`, `StudioApplicationTest`, …) with
`com.mysql.cj.jdbc.exceptions.CommunicationsException: Communications link
failure` — this environment has no MySQL, and none of them is in the ACL layer.
The remaining ~3136 tests pass, including the ones the signature change touches
(`AclControllerTest`, `AclServiceTest`, `AclListToolHandlerTest`).
- The existing tests that call `listRules` were updated mechanically (one
`null` inserted in the new filter position), which is the whole reason the
parameter is grouped with the filters instead of appended.
## Scoring
PRIORITY 68 (impact 26: a visible filter control is inert for every ACL
inventory; reach 14: every instance's ACL rule tab; reproducibility 18:
deterministic slice test, no cluster needed; maintenance value 10: turns an
existing predicate back on). FIX_CONFIDENCE 92.
## Risk
Low. The MySQL path only passes a value the repository already knew how to
filter on; when the console sends nothing (or `all`), the value is `null` and
the query behaves exactly as before — pinned by the second new test. The
Tencent projection is filtered with the same helper and vocabulary as its
sibling filters, so its listing is unchanged when no version is selected. No
schema, response-shape or UI change.
--
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]
