unbridled-41 opened a new pull request, #5119:
URL: https://github.com/apache/rocketmq-dashboard/pull/5119
### Issue linkage
No existing issue or open PR covers this mapping. `gh search prs --repo
apache/rocketmq-dashboard "REQUEST_ACCEPTED"` returns only the merged feature
PR #4455 (`feat(studio): support cloud direct message consumption`); `gh search
issues "REQUEST_ACCEPTED"` returns only the closed feature request #4375. The
earlier defect in this area, #4608 ("Direct message consumption is audited as
SUCCESS even when the consume fails"), was the opposite direction and was fixed
by #4607 (`042b50d9`); this PR does not change that fix's intent — only its
vocabulary gap for the cloud path.
### Problem / Evidence
`MessageService.auditResult`
(`server/src/main/java/org/apache/rocketmq/studio/instance/message/MessageService.java:204-211`)
maps every consume-result spelling except the broker-side `CR_SUCCESS` to
`FAILED`. The Tencent direct-consume path answers `REQUEST_ACCEPTED` after a
successful OpenAPI call:
```java
// TencentInstanceProvider.consumeMessageDirectly
return DirectConsumeMessageResultVO.builder()
.consumeResult("REQUEST_ACCEPTED")
.remark(... "Tencent accepted the verification request; "
+ "the consumption outcome is not returned" ...)
```
`docs/api-spec.md` §8 documents exactly that: Aliyun answers
`CR_SUCCESS`/`CR_FAILED`, while "Tencent 因 OpenAPI 不回传消费结果固定为
`REQUEST_ACCEPTED`，实际语义见 `remark`". The cloud answer is therefore an accepted
operation, not a broker `CMResult` failure — but the audit row recorded it as
one.
Call path: `MessageController.consumeMessageDirectly` ->
`MessageService.consumeMessageDirectly:152` ->
`recordDirectConsumeAudit:181-182` -> `auditResult:205` ->
`rmq_operation_audit.result`.
Observed **before** the fix (the new test, run against the pre-fix revision
of `MessageService`):
```text
Argument(s) are different! Wanted:
operationAuditService.record(
"DIRECT_CONSUME_MESSAGE", "MESSAGE", "msg-1", "tencent-instance",
contains("REQUEST_ACCEPTED"), "SUCCESS", isNull());
Actual invocations have different arguments at position [5]:
operationAuditService.record(
"DIRECT_CONSUME_MESSAGE", "MESSAGE", "msg-1", "tencent-instance",
"topic=orders, consumerGroup=billing, clientId=client-a,
result=REQUEST_ACCEPTED",
"FAILED", null)
```
So an operator who uses 直消费 on a Tencent instance gets a `FAILED` row in the
audit console (and in the result filter's option list) for an operation that
the console itself reported as performed. The same mapping is what writes the
row when the provider really throws, so the two outcomes are indistinguishable
in the audit trail.
Observed **after** the fix: the row is written with `SUCCESS`, and the
detail still carries `result=REQUEST_ACCEPTED`, so the cloud semantics survive
in the row.
### Root cause / Fix
The mapping comment assumed a broker-side `CMResult` spelling on every path;
the cloud providers answer in their own vocabulary. Recognise the documented
accepted spelling as a success, keeping everything else (`CR_FAILED`,
`CR_ROLLBACK`, unknown values) mapped to `FAILED` as before.
### Priority / fix confidence
- PRIORITY = **57** — impact 22/40 (a successful operation is recorded as
failed on a compliance surface, and pollutes the result filter's values), scope
12/20 (Tencent direct consume; the Aliyun path already matches),
reproducibility 18/20 (deterministic unit test), maintenance value 5/20
(corrects a mapping introduced with the cloud consume feature two days before
the base commit).
- FIX_CONFIDENCE = **78** — the accepted spelling is documented in
`docs/api-spec.md` and produced only by a successful OpenAPI response; the
alternative reading ("the audit result means consumption was confirmed") would
need a third audit value, which the schema comment (`result VARCHAR(16) ...
COMMENT 'SUCCESS/FAILED'`) and the console's result filter do not currently
have.
### Tests
| Command | Result |
|---|---|
| `mvn -o -q test -Dtest=MessageServiceTest` | 34 tests, 0 failures |
| `mvn -o -q test -Dtest='org.apache.rocketmq.studio.instance.message.**'` |
61 tests, 0 failures, 1 error |
| `mvn -o test` (whole suite) | Tests run: 3288, Failures: 0, Errors: 17 |
| `mvn -o checkstyle:check` | clean |
The single `instance.message.**` error and the 17 whole-suite errors are the
MySQL-backed integration tests (`QueryHistoryServiceIntegrationTest`,
`*IntegrationTest`, `StudioApplicationTest`) that cannot load a Spring context
in this environment; the errored set equals the set measured on the untouched
base revision `4ee173ad`.
New test:
`MessageServiceTest#auditsACloudAcceptedDirectConsumeAsSuccessTest` (fails
pre-fix with the "Argument(s) are different" output above). The existing
`directsMessageConsumptionThroughSelectedProviderAndAuditsItTest` and
`auditsDirectConsumeFailureWhenConsumeResultIsNotSuccessTest` keep the
`CR_SUCCESS` / `CR_ROLLBACK` mapping pinned.
### Risk
If the maintainers intend the audit result to mean "consumption confirmed"
rather than "the operation succeeded", the correct shape is a distinct stored
value for the accepted-but-unconfirmed outcome instead of `SUCCESS`; that would
change the audit vocabulary (`SUCCESS`/`FAILED` today, plus `PARTIAL` for
cluster updates) and the console's result filter, so it is a product decision
rather than a one-line fix. Everything else in this change is confined to the
two spellings listed above.
--
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]
