You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "Frun1na (via GitHub)" <gi...@apache.org> on 2026/10/15 19:34:32 UTC

[PR] fix(consumer): reject an import batch above the backend group cap [rocketmq-dashboard]

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

   ### Which Issue(s) This PR Fixes
   
   - Fixes #6828
   
   ### Brief Description
   
   The demo data source's `importConsumerGroups` created every row: the mock branch looped over the
   submitted list and called `createConsumerGroup` for each entry
   (`web/src/services/consumerService.ts:323-337` before this change), while the endpoint answers 400
   for more than 100 groups - `MetadataService.importConsumerGroups`
   (`server/src/main/java/org/apache/rocketmq/studio/instance/topic/MetadataService.java:779-781`)
   rejects `size > MAX_IMPORT_GROUPS` (100) with "At most 100 consumer groups are allowed per import",
   and the topic-side import carries the identical cap (`:881-883`).
   
   The console can produce a longer list: the consumer-group CSV import validates each row but has no
   row cap (`web/src/utils/resourceCsvImport.ts:319`), and the page submits every pending row in one
   call (`web/src/pages/instance/consumer.tsx:846-877`). In demo mode a 101-row file was then reported
   as imported row by row (each row marked "已创建" plus a successful-import toast), where the real data
   source fails the whole request and the page's catch branch marks every row with the endpoint's
   message.
   
   The mock branch now throws the endpoint's message before it touches the store; the per-row handling
   (the duplicate-name failures) and the real-API branch are unchanged. This is the same shape the
   alert-rule import already uses (#6784).
   
   User-visible change: in the demo data source, importing more than 100 consumer groups in one batch is
   rejected instead of adding the rows. No copy is added; no production path is affected.
   
   ### Other consumers
   
   - `web/src/services/consumerService.ts:319` `importConsumerGroups` - fixed here; it is the only consumer-group import surface in the web layer.
   - `web/src/services/topicService.ts` `importTopics` - the same mock gap (the endpoint caps topics at 100 too, `MetadataService.java:881-883`); deliberately left to a separate change in the topic service: the two files carry independent open changes and this PR is scoped to the consumer-group import.
   - The endpoint's two other import checks are not mirrored: a blank `instanceId` and an empty list ("groups is required"). The page guarantees both before calling (`web/src/pages/instance/consumer.tsx:824-840` requires a selected instance and at least one pending row), so no reachable copy exists to test.
   - `web/src/utils/resourceCsvImport.ts:319` - the per-row validator stays as it is: it guards the rows the operator uploaded (console-side), while the batch cap is a contract the endpoint enforces in its service, so it belongs at the service boundary - the same layering as #6784.
   - The other mock paths in this file are untouched: the delete family is #6351, the CSV columns are #6001/#5920, and the subscription-mode filter is #6172.
   
   ### How Did You Test This Change?
   
   ```
   $ cd web && npx eslint src/services/consumerService.test.ts src/services/consumerService.ts
   # clean
   
   $ cd web && npx prettier --check src/services/consumerService.test.ts src/services/consumerService.ts
   Checking formatting...
   All matched files use Prettier code style!
   
   $ cd web && npx vitest run src/services/consumerService.test.ts --maxWorkers=1 --fileParallelism=false
    ✓ src/services/consumerService.test.ts (28 tests) 49ms
    Test Files  1 passed (1)
         Tests  28 passed (28)
   
   $ cd web && npx vitest run src/pages/instance/__tests__/ConsumerPage.test.tsx --maxWorkers=1 --fileParallelism=false
    Test Files  1 passed (1)
         Tests  56 passed (56)
   
   $ cd web && npx tsc -b
   # clean
   ```
   
   The new test fails without the fix (this is the mutation check: `web/src/services/consumerService.ts`
   is restored to the version without the guard, the test file is kept):
   
   ```
   $ cd web && npx vitest related --run src/services/consumerService.test.ts --maxWorkers=1 --fileParallelism=false
    FAIL  src/services/consumerService.test.ts > consumer group import batch contract > rejects an import above the backend batch cap
   AssertionError: promise resolved "{ imported: 101, failed: +0, …(2) }" instead of rejecting
   
    ❯ src/services/consumerService.test.ts:579:66
       577|     // list row by row.
       578|     const batch = batchOf('mock-import-over-cap', 101);
       579|     await expect(importConsumerGroups('instance-direct-1', batch)).rej…
          |                                                                  ^
   
    Test Files  1 failed (1)
         Tests  1 failed | 27 passed (28)
   ```
   
   The assertion is that a 101-row batch rejects with the endpoint's message and that none of those rows
   reaches the demo inventory, so it can only pass once the mock refuses the batch the endpoint refuses;
   the received value in the mutation output (`imported: 101`) is exactly the row-by-row import. The
   second test doubles as the control at the boundary: exactly 100 rows still import and are cleanly
   removed afterwards, and the 26 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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(consumer): reject an import batch above the backend group cap [rocketmq-dashboard]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6829:
URL: https://github.com/apache/rocketmq-dashboard/pull/6829#issuecomment-6355254152

   This pull request has been automatically closed because there has been no activity for 7 days. If you would like to continue working on this, please reopen the PR with updated changes.


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(consumer): reject an import batch above the backend group cap [rocketmq-dashboard]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #6829: fix(consumer): reject an import batch above the backend group cap
URL: https://github.com/apache/rocketmq-dashboard/pull/6829


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org