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/01 15:22:28 UTC

[PR] fix(topic): require the queue counts on topic updates [rocketmq-dashboard]

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

   ### Which Issue(s) This PR Fixes
   
   None — this implements exactly the design lizhimins prescribed in the review on #4581 ("make the
   counts required on the update DTO (they are already `Integer` + `@PositiveOrZero`) and confine the
   default-8 substitution to `createTopic`, where it belongs"), so it reuses that review as its context
   instead of opening a duplicate issue. The underlying drain-to-zero defect itself is unchanged from
   that PR: an update DTO without queue counts reaches the broker as 0, is read as "not provided", and
   is refilled from the stored value or the default of 8.
   
   ### Which Issue(s) This PR Fixes
   
   - Fixes #5274
   
   ## Brief Description
   
   Following the prescribed boundary design:
   
   - `UpdateTopicDTO.writeQueues` / `readQueues` are now `@NotNull` (still `@PositiveOrZero`, so an
     explicit **0 is accepted** and drains the topic), and `toTopicVO()` copies them unconditionally.
   - `RocketMQAdminClientImpl.updateTopic` sends the caller's value as-is: the
     "stored value, else 8" fallback chain is gone, and with it the only place where a number other
     than the caller's choice could be written to the broker.
   - The default-8 substitution stays confined to `createTopic`.
   - The REST contract already documents both counts as required (`docs/api-spec.md` §5.4 marks
     `writeQueues` / `readQueues` 必填), so the code now matches the documented contract.
   
   `TopicUpdateInput.mergeWith` (the AI tool path) fills absent counts from the current topic, so
   partial updates through the agent keep writing the merged values.
   
   ### How Did You Test This Change?
   
   ```
   $ mvn -B -ntp -Dtest='RocketMQAdminClientImplTest,TopicControllerTest,MetadataServiceTest,TopicMutationPlanTest' \
         -DfailIfNoTests=false test
   [INFO] Tests run: 147, Failures: 0, Errors: 0, Skipped: 0
   [INFO] BUILD SUCCESS
   ```
   
   - New `updateTopicAppliesExplicitZeroQueueCountsToDrainATopic` feeds an explicit 0/0 and asserts the
     broker `TopicConfig` and the stored row both see 0. It fails on trunk with `expected: 0 but was: 8`.
   - New controller test: `POST /api/topics/update` with `{"name":"orders"}` returns 400
     `writeQueues is required`.
   - `updateTopicPreservesQueueCountsWhenNotSpecified` was removed: its premise (a perm-only partial
     update reaching the admin client without counts) no longer exists under the required contract.
   - The now-unused `TopicPerm` import was dropped (checkstyle).
   
   ### 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`
   - [ ] New UI text has both Chinese and English entries under `web/src/i18n/`
   - [x] 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)
   
   
   ---
   
   Re-submission of #4772: the original PR was closed by the stale bot after 7 days without review activity, and GitHub now rejects reopening pull requests in this repository ("state cannot be changed"), so the same change is re-submitted rebased onto the current `rocketmq-studio` tip (`a460673f`). The original review thread stays available at #4772.
   


-- 
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(topic): require the queue counts on topic updates [rocketmq-dashboard]

Posted by "Frun1na (via GitHub)" <gi...@apache.org>.
Frun1na commented on PR #5275:
URL: https://github.com/apache/rocketmq-dashboard/pull/5275#issuecomment-6044107931

   Friendly ping — this is still open for review whenever you get a chance. (Keeping it from the stale bot for now.)


-- 
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(topic): require the queue counts on topic updates [rocketmq-dashboard]

Posted by "Frun1na (via GitHub)" <gi...@apache.org>.
Frun1na commented on PR #5275:
URL: https://github.com/apache/rocketmq-dashboard/pull/5275#issuecomment-6093175977

   Friendly ping — this is still open for review whenever you get a chance. (Keeping it from the stale bot for now.)


-- 
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