Wang1rrr opened a new pull request, #4854:
URL: https://github.com/apache/rocketmq-dashboard/pull/4854
<!-- Make sure the base branch is `master`: that is the RocketMQ Studio
trunk. -->
### Which Issue(s) This PR Fixes
- Fixes #4851
### Brief Description
`RocketMQMessageProvider` has carried a private, 41-line fallback for
building a message's consumer status from the broker since #798, and no request
has ever reached it:
```java
private List<ConsumerStatusVO> fallbackConsumerStatus(DefaultMQAdminExt
adminExt, MessageExt message) { ... }
private DeliveryStatus mapTrackType(TrackType trackType) { ... }
```
`fallbackConsumerStatus` has no caller in `server/src` (main or test) and
`mapTrackType` is called only from inside it. It was already caller-less in the
commit that introduced it (`dc49e1a2`) and survived `ceef946f` (#1231)
unchanged. The live path builds consumer status from parsed trace-topic records
instead (`buildConsumerStatus`, `:777`).
The reason this is worth removing rather than leaving alone is that the dead
mapping is wrong, and it reads exactly like implemented behaviour:
| `TrackType` | mapped to | means |
| --- | --- | --- |
| `CONSUMED`, `CONSUME_BROADCASTING`, `CONSUMED_BUT_FILTERED` | `success` |
delivered |
| `NOT_CONSUME_YET`, `PULL`, `NOT_ONLINE` | `pending` | not delivered yet /
no consumer online |
| `null` | `pending` | could not tell |
| **`UNKNOWN`** (via `default`) | **`failed`** | **could not tell** |
`TrackType` has exactly seven members in rocketmq-tools 5.5.0, so `UNKNOWN`
is the only value that reaches `default` - and
`DefaultMQAdminExtImpl.messageTrackDetail` sets `UNKNOWN` as the *initial*
value on every group's `MessageTrack` before it queries anything, replacing it
with `NOT_ONLINE` only when the broker answers 206 (`CONSUMER_NOT_ONLINE`). Any
other failure - unreachable broker, timeout, `RemotingException` - leaves it at
`UNKNOWN`.
So the same "we could not determine this" state is `pending` when spelled
`null` and `failed` when spelled `UNKNOWN`. Wired in as written, that would put
a red 失败 (`DELIVERY_STATUS_MAP`, `web/src/pages/instance/message.tsx:108`) on
**every** consumer group of the topic, on exactly the clusters where the admin
RPCs are failing.
**The change** deletes both methods and the two imports that become unused
(`org.apache.rocketmq.tools.admin.api.MessageTrack` and `TrackType`). No
behaviour changes, because nothing could call them.
If the community would rather have the fallback than not, that is a separate
and deliberate change, and the issue lists what it needs: `UNKNOWN` mapped to
`pending` (matching the `null` case here and the `pending` fallback
`AliyunConverters:327` already uses), a decision on the cost -
`messageTrackDetail` issues one `queryTopicConsumeByWho` plus one
`examineConsumerConnectionInfo` **per subscribed group**, from the
`TOPIC_NOT_EXIST` / `NO_MESSAGE` branch at `:600`, i.e. the slow path of a page
already blocked on RPCs - and a way to get the `MessageExt`, which
`resolveMessageStoreTimestamp` (`:670`) already fetches and discards. I am
happy to send that instead if it is wanted; this PR just stops shipping the
trap.
### How Did You Test This Change?
```
cd server
mvn -o -B test "-Dtest=RocketMQMessageProvider*,Message*Test,*TraceTest"
Tests run: 137, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
```
That covers `RocketMQMessageProviderTest` (56), `MessageServiceTest`,
`MessageControllerTest` and the seven message/trace AI tool handler suites.
Checkstyle runs in the `validate` phase (`failsOnError=true`,
`includeTestSourceDirectory=true`), so the build also proves no unused import
or reference survived.
Reachability was established before deleting, not assumed:
```
rg -n "fallbackConsumerStatus|mapTrackType|messageTrackDetail" server/src
# three hits, all inside the two methods themselves
```
and the removal script asserts that none of `MessageTrack`, `TrackType`,
`fallbackConsumerStatus`, `mapTrackType` or `messageTrackDetail` appears
anywhere in the file afterwards, so a half-deletion cannot slip through.
Checkstyle's `UnusedImports` is the second net: leaving the imports behind
fails `validate` (confirmed while preparing this change).
No new tests: there is no reachable behaviour left to pin. The trace parsing
that *is* live stays covered by the existing `RocketMQMessageProviderTest`
cases, all of which pass unchanged.
### 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` - none added; this deletes unreachable private code and the 137
existing message/trace tests pass unchanged
- [x] New UI text has both Chinese and English entries under `web/src/i18n/`
(no UI change)
- [x] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks)
- [x] New source files carry the ASF license header (no new source files)
- [x] Documentation touched where behaviour changed (README / `docs/` /
in-app help) - none needed; no reachable behaviour changed, so no documented
contract did either
--
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]
