iloveeyjafjalla opened a new pull request, #11370:
URL: https://github.com/apache/rocketmq/pull/11370
### Which Issue(s) This PR Fixes
- Fixes #11369
### Brief Description
`QueryMessage` accepted the request's pagination fields and then ignored
them:
- `page_size` was never read (`maxNums` came from `max_message_nums` alone,
defaulting to 32);
- `page_num` was never read, so page 2 answered with the same first page
again;
- `scroll_id` was only echoed into the response, advertising a cursor the
proxy never issues, so passing it back could never
continue anything.
The broker exposes no ordering and no cursor for this search, so the change
honors what can be honored and refuses the rest:
- `page_size` (when set) caps the query — `maxNums = min(max_message_nums,
page_size)` — and a non-positive `page_size` is
rejected as `BAD_REQUEST`;
- `page_num > 1` is rejected as `BAD_REQUEST` (a page past the first cannot
be produced);
- a non-empty `scroll_id` is rejected as `BAD_REQUEST`, and the response no
longer carries a `scroll_id`.
That is the rule this RPC already applies to its unsupported `search_key`
variants, and `docs/proxy-admin.md` /
`docs/proxy-admin-zh.md` §8 now record the boundary.
### How Did You Test This Change?
New tests in `ProxyAdminGrpcServiceTest`:
- `queryMessagePageSizeCapsResultsTest`: two messages available, `page_size
= 1` → exactly one message is returned **and**
the broker is queried with `maxNum = 1` (the cap is pushed into the query,
not applied to its answer).
- `queryMessageRejectsPageNumBeyondTheFirstPageTest` /
`queryMessageRejectsScrollIdTest`: `page_num = 2` and
`scroll_id = "cursor-1"` are refused with `BAD_REQUEST` before any route
or broker lookup.
- `queryMessageRejectsNonPositivePageSizeTest`: `page_size = 0` →
`BAD_REQUEST`.
Results on Temurin JDK 8 (same JDK as CI), base `9ea2ccdc2` plus this patch:
- before the fix: `expected:<1> but was:<2>` (page_size ignored),
`expected:<BAD_REQUEST> but was:<OK>` (page_num),
`expected:<BAD_REQUEST> but was:<OK>` (scroll_id)
- after the fix: `ProxyAdminGrpcServiceTest — Tests run: 64, Failures: 0,
Errors: 0, Skipped: 0`
- `mvn -B -ntp -pl proxy package` with checkstyle + spotbugs enabled: `BUILD
SUCCESS`, spotbugs `BugInstance size is 0`
This change was prepared with AI assistance (analysis, patch and test
drafting); the diff and every result above were
reproduced locally by running the module's tests against the base commit and
the patched tree.
--
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]
