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

   ### Which Issue(s) This PR Fixes
   
   - Fixes #6824
   
   ### Brief Description
   
   The demo data source's `createInstance` stored whatever it was handed: the 
mock branch built the row
   with `name: data.name || ''` and `endpoint: data.endpoint || ''` and pushed 
it
   (`web/src/services/instanceService.ts:90-102` before this change), so a 
whitespace-only name or
   endpoint was registered. The endpoint answers 400 for the same input: a 
manual instance goes through
   `InstanceService.createApacheInstance`, which normalizes the name with 
`requireInstanceName`
   
(`server/src/main/java/org/apache/rocketmq/studio/instance/InstanceService.java:590-599`,
 blank after
   trimming -> "InstanceVO name is required", stores the trimmed value) and the 
endpoint with
   `requireValidEndpoint` (`:577-588`, blank -> "InstanceVO endpoint is 
required", stores the trimmed
   value).
   
   The registration dialog can produce both inputs: the name input declares 
only `required` and
   `max: 64` and the endpoint input only `required`
   (`web/src/pages/instance/index.tsx:958-965`, `:987-992`), and the Ant 
validator accepts
   whitespace-only text, which `handleCreate` submits unchanged (`:364-382`). 
In demo mode the row then
   appeared in the instance list with a blank name - and since the instance 
routes, selectors and every
   instance-scoped request are keyed by that name, the blank value travelled 
into them - where the real
   data source takes the page's existing error branch.
   
   The mock branch now runs the same two checks (blank rejected with the 
endpoint's message, value
   stored trimmed) through `requireMockInstanceText`; the cloud path keeps its 
catalog-derived
   name/endpoint behaviour. The inflight-list invalidation and the created-row 
copy are unchanged.
   
   User-visible change: in the demo data source a blank instance name or 
endpoint is rejected instead
   of being registered, and the stored values are trimmed like the backend's. 
No copy is added; no
   production path is affected.
   
   ### Other consumers
   
   - `web/src/services/instanceService.ts:87` `createInstance` - fixed here; it 
is the only instance registration path in the web layer.
   - `web/src/services/instanceService.ts` `updateInstance` - the edit dialog 
renders the name as a disabled input 
(`web/src/pages/instance/index.tsx:1026-1029`) and the endpoint does not accept 
a blank endpoint on update, so no reachable copy; unchanged.
   - The endpoint's remaining field rules are deliberately not mirrored: the 
name length (64) is already enforced by the dialog's `max: 64`, and the 
endpoint length/empty-address bounds (`requireValidEndpoint` after the blank 
check) are a length/format rule family of their own. The mock store has no 
column widths to protect.
   - Duplicate instance names are not part of this change: the sequential 
duplicate is a separate registration rule, and its concurrent variant is 
already tracked in #5296.
   - Cloud (`ALIYUN`/`TENCENT`) creates keep the mock's old behaviour: the 
console's credential picker has no demo data source 
(`web/src/api/cloudCredential.ts` has no service wrapper), so that branch is 
not reachable in demo mode.
   
   ### How Did You Test This Change?
   
   ```
   $ cd web && npx eslint src/services/instanceService.test.ts 
src/services/instanceService.ts
   # clean
   
   $ cd web && npx prettier --check src/services/instanceService.test.ts 
src/services/instanceService.ts
   Checking formatting...
   All matched files use Prettier code style!
   
   $ cd web && npx vitest run src/services/instanceService.test.ts 
--maxWorkers=1 --fileParallelism=false
    ✓ src/services/instanceService.test.ts (17 tests) 15ms
    Test Files  1 passed (1)
         Tests  17 passed (17)
   
   $ cd web && npx vitest run 
src/pages/instance/__tests__/InstancePage.test.tsx --maxWorkers=1 
--fileParallelism=false
    Test Files  1 passed (1)
         Tests  34 passed (34)
   
   $ cd web && npx tsc -b
   # clean
   ```
   
   The new tests fail without the fix (this is the mutation check: 
`web/src/services/instanceService.ts`
   is restored to the version without the guard, the test file is kept):
   
   ```
   $ cd web && npx vitest related --run src/services/instanceService.test.ts 
--maxWorkers=1 --fileParallelism=false
    FAIL  src/services/instanceService.test.ts > instanceService mock instances 
> rejects the blank name and endpoint the API requires
   AssertionError: promise resolved "{ id: 1792091724044, name: '   ', …(8) }" 
instead of rejecting
   
   - Error {
   -   "message": "rejected promise",
   + {
   +   "consumerGroupCount": 0,
   +   "endpoint": "10.0.1.90:9876",
   +   "gmtCreate": "2026-10-15 19:15:24",
   +   "gmtModified": "2026-10-15 19:15:24",
   +   "id": 1792091724044,
   +   "name": "   ",
   +   ...
   + }
   
    FAIL  src/services/instanceService.test.ts > instanceService mock instances 
> stores the trimmed name and endpoint the API stores
   AssertionError: expected '  mock-trimmed-create  ' to be 
'mock-trimmed-create'
   
    Test Files  1 failed (1)
         Tests  2 failed | 15 passed (17)
   ```
   
   The first assertion is that a blank name/endpoint rejects and 
`listInstances()` stays unchanged, so
   it can only pass once the mock refuses the values the endpoint refuses; the 
received object in the
   output is the nameless row the store used to keep. The trim test doubles as 
the control: it would
   fail if the guard were widened into a rejection of padded values, and the 
fifteen neighbouring cases
   pin the rest of the service.
   
   ### 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]

Reply via email to