Wang1rrr opened a new pull request, #4857:
URL: https://github.com/apache/rocketmq-dashboard/pull/4857
<!-- Make sure the base branch is `rocketmq-studio`: that is the RocketMQ
Studio trunk. -->
### Which Issue(s) This PR Fixes
- Fixes #4856
### Brief Description
`AliyunConverters.toMessageRecord` published a third `bodyEncoding` value
that the rest of Studio does not define, and attached it to the wrong payload.
`docs/api-spec.md:1335` documents the field as `UTF-8` / `BASE64`,
`RocketMQMessageProvider.displayBody`
(`provider/apache/RocketMQMessageProvider.java:881`) emits exactly those two
plus `null`, and `TencentInstanceProvider.toRecordVO`
(`provider/tencent/TencentInstanceProvider.java:808`, `:828`) emits `UTF-8` /
`null`. The Aliyun path emitted `"TEXT"` - and put the **raw Base64** into
`body` while doing so.
The cause is that `tryBase64Decode` collapsed three distinct outcomes into
one `null`:
| # | situation | old result | correct result |
| --- | --- | --- | --- |
| 1 | API returned no body | `body=null`, `"TEXT"` | `body=null`, `null` |
| 2 | Base64 of a **binary** payload | `body=<base64>`, `"TEXT"` |
`body=<base64>`, `"BASE64"` |
| 3 | body is not Base64 at all | `body=<raw>`, `"TEXT"` | `body=<raw>`,
`"UTF-8"` |
Case 2 is the one `BASE64` exists for, and case 1 should carry no encoding -
an encoding for a value that is not there.
**The change** keeps the three apart instead of flattening them:
```java
private static Body decodeBody(String raw) {
if (raw == null || raw.isBlank()) {
return null; // case 1: no body,
no encoding
}
byte[] bytes;
try {
bytes = Base64.getDecoder().decode(raw);
} catch (IllegalArgumentException ignored) {
return new Body(raw, "UTF-8"); // case 3: already
literal text
}
try {
String text = StandardCharsets.UTF_8.newDecoder()
.onMalformedInput(CodingErrorAction.REPORT)
.onUnmappableCharacter(CodingErrorAction.REPORT)
.decode(ByteBuffer.wrap(bytes))
.toString();
return new Body(text, "UTF-8"); // text payload
} catch (CharacterCodingException ignored) {
return new Body(raw, "BASE64"); // case 2: binary,
Base64 is faithful
}
}
```
and `toMessageRecord` just forwards the pair:
```java
Body body = decodeBody(data.getBody());
...
if (body != null) {
builder.body(body.value()).bodyEncoding(body.encoding());
}
```
This is the same shape `RocketMQMessageProvider.displayBody` already uses
(strict UTF-8 decode, fall back to Base64 with a `BASE64` label), so the three
providers now agree on the vocabulary and a consumer that switches on the
documented values takes the same branch regardless of vendor.
Why it mattered beyond tidiness: `MessageItem.from`
(`ops/ai/tool/contract/message/MessageItem.java:46`) and `MessageQueryOutput`
(`:76`) forward the field verbatim, and the tool schema
(`resources/tool-catalog/tools/message.yaml:86`, `:171`, `:254`) declares it as
a bare `type: string` with no enum, so `rmq.message.query*` told the model
`{"body": "<base64>", "bodyEncoding": "TEXT"}` for every binary message on an
Aliyun instance - an agent reads that as literal text and quotes the Base64
back to the operator as the message content. `rmqctl` advertises the same
fields (`internal/catalog/catalog_gen.go:389`, `:410`, `:419`).
Deliberately out of scope: `web/src/pages/instance/message.tsx` never
consults `bodyEncoding` - `formatBody` (`:140`) only tries `JSON.parse` - so a
`BASE64` body from *any* provider is displayed and downloaded as raw Base64.
That belongs in the web layer and is a separate change.
### How Did You Test This Change?
```
cd server
mvn test -Dtest=Aliyun*
Tests run: 68, Failures: 0, Errors: 0, Skipped: 0
You have 0 Checkstyle violations.
BUILD SUCCESS
mvn test -Dtest=*Message*,*Trace*,*Dlq*,*DLQ*
Tests run: 226, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
```
Checkstyle runs in the `validate` phase of this build (`failsOnError=true`,
`includeTestSourceDirectory=true`), so the run also proves the new code and
test pass `style/rmq_checkstyle.xml`.
New test class `AliyunConvertersBodyEncodingTest` (5 cases), one per row of
the table above plus the invariant:
- `shouldReportUtf8WhenTheBase64BodyDecodesToText`
- `shouldReportBase64WhenThePayloadIsBinary` - Base64 of `0xFF 0xFE`,
asserts the body stays the Base64 and the label becomes `BASE64`
- `shouldKeepLiteralTextWhenTheBodyIsNotBase64` - `"{}"` keeps its own value
and is labelled `UTF-8`
- `shouldReportNoEncodingWhenTheApiReturnedNoBody` - `null` and blank both
yield `null`/`null`
- `shouldNeverEmitAnEncodingOutsideThePublishedVocabulary` - feeds a text
body, a binary body, a non-Base64 string, an empty string and `null` through
the converter and asserts every result is `isIn("UTF-8", "BASE64", null)`, so a
fourth label cannot be reintroduced
One existing assertion updated:
`AliyunInstanceProviderTest.queryMessagesShouldMapFieldsAndDecodeBase64BodyTest:544`
pinned `"TEXT"` for a `"{}"` body. That fixture is case 3, so it now asserts
`"UTF-8"`; the `body` assertion (`"{}"` passthrough) is untouched, which is the
behaviour that test was really about.
Mutation-checked against the current trunk: restoring only the old `else {
builder.body(rawBody).bodyEncoding("TEXT"); }` branch makes `mvn test
-Dtest=AliyunConvertersBodyEncodingTest` report `Tests run: 4, Failures: 3`
(`shouldReportBase64WhenThePayloadIsBinary`,
`shouldReportNoEncodingWhenTheApiReturnedNoBody`,
`shouldNeverEmitAnEncodingOutsideThePublishedVocabulary`), and the reverted
converter also fails the updated `AliyunInstanceProviderTest` assertion. With
the fix in place all 68 Aliyun tests and all 226 message/trace/DLQ tests are
green.
### 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
`...` (the new converter tests follow the existing `AliyunConverters*Test`
naming, which does not use the `...Test` method suffix; the updated provider
test keeps its `...Test` suffix)
- [x] New UI text has both Chinese and English entries under `web/src/i18n/`
(no new UI text)
- [x] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks)
- [x] New source files carry the ASF license header
(`AliyunConvertersBodyEncodingTest.java` does)
- [x] Documentation touched where behaviour changed (README / `docs/` /
in-app help) - none needed; `docs/api-spec.md:1335` already documents `UTF-8` /
`BASE64`, and this change is what makes the Aliyun provider honour it
--
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]
