Frun1na opened a new pull request, #6784:
URL: https://github.com/apache/rocketmq-dashboard/pull/6784
### Which Issue(s) This PR Fixes
- Fixes #6610
### Brief Description
The Mock branch of `importAlertRulesTransfer`
(`web/src/services/opsService.ts:177`) mapped the document straight onto the
inventory:
```ts
if (!isMockMode()) return opsApi.importAlertRulesTransfer(data, domain);
const startId = Date.now();
const imported = data.rules.map((rule, index) => ({ ...copyAlertRule(rule as
AlertRule), id: startId + index }));
alertRulesState[domain].push(...imported);
```
Nothing checked the envelope, so the demo data source accepted four
documents the endpoint answers 400 for.
`AlertRuleTransferService.validateEnvelope`
(`server/.../ops/alert/AlertRuleTransferService.java:51-64`) requires `version
== 1`, the domain of the page it is imported from, and between 1 and
`MAX_RULES_PER_IMPORT` (200) rules. An empty `rules` array therefore reported a
successful import of zero rules, 201 rules were appended, a document for the
other domain was appended to the domain passed in, and a `version: 2` document
was accepted.
The page does not close the gap: `handleImportRules` checks only version,
domain and `Array.isArray` (`web/src/pages/ops/alerts.tsx:456`), so the
file-upload path reaches the service with an empty or oversized document and
the operator is told it imported.
The Mock branch now performs the endpoint's four checks with the endpoint's
messages before it touches the inventory. The real-API branch is untouched.
User-visible change: in demo mode an empty or oversized import document now
takes the page's existing `alerts.importFailed` notice instead of reporting an
import, and a document for another domain is no longer written into the wrong
domain's inventory.
### Other consumers
- `web/src/services/opsService.ts:177` `importAlertRulesTransfer` — fixed
here. The Mock branch is the only mock surface for alert-rule import.
- `web/src/services/opsService.ts:168` `exportAlertRulesTransfer` — produces
`{version: 1, domain, rules}` and reads no envelope; unchanged, and its output
still round-trips through the new checks.
- `web/src/pages/ops/alerts.tsx:456` — the page's own pre-check still runs
first for the file path and is deliberately left as it is: it guards the JSON
shape the operator uploaded, while the rule count is a contract the endpoint
enforces in its service (not in the DTO), which is why the same check belongs
at the service boundary. The page's `catch` already routes both to
`alerts.importFailed`, so no new UI text is needed.
- `server/.../ops/alert/AlertRuleTransferDTO.java` — `@NotEmpty` on `rules`
and `@Valid` on each entry; the 200-rule ceiling lives in
`AlertRuleTransferService`. Nothing to change; the Mock now mirrors both layers.
- `server/.../ai/tool/**` — no AI tool imports alert rules (`git grep -n
AlertRuleTransferDTO -- server/src/main/java/org/apache/rocketmq/studio/ops/ai`
is empty), so no catalog or tool-output surface consumes this envelope.
- Per-rule field validation (the endpoint's `@Valid` plus
`metricCatalogService.validate`) is a separate surface and is out of scope
here: a rule whose `channels` are missing already fails loudly in the Mock, and
this PR is about the envelope.
### How Did You Test This Change?
```
$ ./wb test --filter src/services/opsService.test.ts
✗ 2.1s npx vitest run src/services/opsService.test.ts --maxWorkers=1
--fileParallelism=false
+ Received:
[]
❯ src/services/opsService.test.ts:204:5
203| importAlertRulesTransfer({ version: 1, domain:
'CLUSTER', rules: …
204| ).rejects.toThrow('Alert rule import must contain between
1 and 20…
Test Files 1 failed (1)
Tests 1 failed | 17 passed (18)
$ ./wb test
✓ 1.3s npx eslint src/services/opsService.test.ts
src/services/opsService.ts
✓ 0.6s npx prettier --check src/services/opsService.test.ts
src/services/opsService.ts
✓ 94.8s npx vitest related --run src/services/opsService.test.ts
src/services/opsService.ts
Test Files 8 passed (8)
Tests 121 passed (121)
✓ 4.3s npx tsc -b
```
The new test fails without the fix:
```
# only the thirteen validation lines removed from the Mock branch, the tests
kept
$ ./wb test --filter src/services/opsService.test.ts
✗ 2.0s npx vitest run src/services/opsService.test.ts --maxWorkers=1
--fileParallelism=false
+ Received:
[]
Test Files 1 failed (1)
Tests 1 failed | 17 passed (18)
✓ 3.0s npx tsc -b
```
The assertion goes red because it asks an empty envelope to reject; without
the guard `[].map()` yields `[]`, the push adds nothing, and the call resolves
with `[]` — the failure reads `Received: []`, i.e. an empty document reported
as a successful import of zero rules. Its sibling in the same test file, `still
imports the alert rule counts the endpoint accepts Test`, imports one rule and
then two hundred and passes both with and without the guard, which is what
shows the assertions pin the envelope rather than "the function throws".
The report in #6610 and the earlier attempt in #6611 (closed on 2026-10-15
by the seven-day inactivity sweep, not on review) describe the same defect;
this change is scoped to the envelope the issue names.
### 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`
- [ ] New UI text has both Chinese and English entries under `web/src/i18n/`
- [ ] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks)
- [ ] New source files carry the ASF license header
- [ ] Documentation touched where behaviour changed (README / `docs/` /
in-app help)
--
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]
