unbridled-41 opened a new pull request, #3999:
URL: https://github.com/apache/rocketmq-dashboard/pull/3999

   Fixes #3998.
   
   ## Source
   
   - Parent project (classic dashboard, default branch `master`): 
[`frontend-new/src/components/DlqMessageDetailViewDialog.jsx`](https://github.com/apache/rocketmq-dashboard/blob/master/frontend-new/src/components/DlqMessageDetailViewDialog.jsx)
 renders the properties payload of a dead-letter message (properties JSON plus 
TAGS/KEYS) in the DLQ detail dialog.
   - In-repo precedent that DLQ properties matter here: PR #1436 (merged, 
"preserve DLQ properties and classify resend outcomes") made DLQ resend 
preserve business user properties — the display path is the missing counterpart.
   - Feature request filed for this gap: #3998.
   
   ## Current gap
   
   On the base commit (36126024), the DLQ message drawer 
(`web/src/pages/instance/dlq.tsx`, served by `GET 
/api/dlq/{groupName}/messages`) shows msgId, topic, queue, offset, store time, 
keys and a body preview — no user properties anywhere. The Apache provider 
builds every `DLQMessageVO` from the full `MessageExt` it has already scanned 
(`RocketMQDLQProvider.toExportVO`, used by both `listMessages` and the 
exports), and `DLQMessageVO` has no properties field, so data already in memory 
is discarded. Verified by reading the builder and by the red tests below. Not 
an intentional omission: the DLQ surface has been actively extended 
(resend-selected, Excel export, time-range resend) without any statement 
excluding property display, and the classic console shows exactly this data.
   
   ## Project fit
   
   Zero extra broker cost: the scan already pulls full `MessageExt` records per 
request, so this is a pure "stop dropping fetched data" change — the same class 
as the accepted property-display gap #3282. The UI reuses the DLQ drawer's 
existing expandable-row idiom (the same `expandedRowRender` pattern the 
consumer page uses), and the payload contract mirrors the message explorer's 
existing 64-entry / 1024-char property limits so both surfaces behave 
identically.
   
   ## Scope
   
   Included:
   
   - `DLQMessageVO` gains `properties` (bounded map) and `propertiesTruncated`; 
the Apache DLQ provider fills both from `message.getProperties()` in 
`toExportVO` (covering the drawer list and both export paths).
   - The provider applies the message explorer's property contract (64 entries 
max, 1024 chars per value, alphabetical order, `...` suffix) and sets 
`propertiesTruncated` when entries were dropped or any value was oversized.
   - The DLQ drawer's message table gains expandable rows rendering the 
property key/value list, with a truncation notice when `propertiesTruncated` is 
set and a "no user properties" fallback otherwise.
   
   Not included: DLQ Excel/CSV export columns (the export row type stays as 
is), changes to the resend flow (already property-preserving since #1436), 
cloud providers (only `RocketMQDLQProvider` implements DLQ listing today), and 
an extra detail modal (the expandable row keeps the drawer's one-level 
structure).
   
   ## Implementation
   
   - `DLQMessageVO`: +`Map<String, String> properties`, +`boolean 
propertiesTruncated`.
   - `RocketMQDLQProvider`: +`MAX_PROPERTIES`/`MAX_PROPERTY_VALUE_CHARS` 
constants mirroring `RocketMQMessageProvider`, +private 
`limitProperties`/`hasOversizedProperty`/`abbreviate` helpers, `toExportVO` 
fills both fields.
   - Web: `DLQMessage` type +2 optional fields; the drawer table gains an 
`expandable.expandedRowRender` following the consumer page's existing pattern.
   
   ## Tests (actual commands and results)
   
   - `listMessagesShouldCarryLimitedUserProperties` 
(`RocketMQDLQProviderTest`): a scanned dead letter with user properties yields 
a VO whose `properties` map carries them and `propertiesTruncated=false`.
   - `listMessagesShouldFlagTruncatedPropertyPayloads`: a 1500-char property 
value is clipped to 1024 chars + `...` and `propertiesTruncated=true`.
   - `shows user properties in the DLQ message drawer` (`DLQPage.test.tsx`): 
opening the drawer and expanding a message row renders the property keys/values.
   - Red (implementation stashed, tests kept): server `mvn test-compile` → 
`cannot find symbol` on 
`getProperties()`/`getPropertiesTruncated()`/`isPropertiesTruncated()` (4 
errors); web `npx vitest run -t "user properties"` → 
`TestingLibraryElementError: Unable to find an accessible element with the role 
"button" and name /expand/i`.
   - Green: `mvn -ntp test -Dtest=RocketMQDLQProviderTest` → **Tests run: 34, 
Failures: 0**; `npx vitest run src/pages/instance/__tests__/DLQPage.test.tsx` → 
**19 passed (19)**.
   - Full backend `mvn -ntp clean test` → **2037 tests, 3 failures**, 
byte-identical to the pristine baseline of 36126024 (AuthCorsIntegrationTest 
×2, AliyunInstanceProviderTest.getGroupProgressShouldMapLagRowsTest); 2037 = 
2035 baseline + 2 new tests; zero new failures.
   - Full web `npx vitest run` → **923 tests, 5 failures**, all five in 
untouched files (ClusterPage ×1, ConsumerPage ×3, TopicPage ×1) while the 
backend suite ran in parallel; those four files pass 91/91 in isolation once 
the load clears — the documented load fragility of the full web suite in this 
sandbox; 923 = 922 + 1 new test.
   - `npx tsc -b` clean; `eslint` clean on the changed files (the one warning 
on `dlq.tsx` line 81 is pre-existing on the base, verified by stashing); `npm 
run build` succeeds.
   - Not executable here: upstream CI (head 
`f77c10a2c4b9dfbc539f361b8405cea2eebabf04` has 0 check runs / no workflow runs; 
the workflow currently fails to start for all branches) — results above are 
from local execution.
   
   ## Compatibility & Risk
   
   - Additive: optional web fields, VO fields defaulted to null/false, one 
provider-scoped helper set. No DB, dependency, or license impact; only rocketmq 
client types already in use.
   - Property values are bounded exactly like the message explorer (64 entries 
/ 1024 chars), so no unbounded payload reaches the browser; binary DLQ messages 
keep the existing `body`/`bodyBase64` behavior untouched.
   - Risk: the drawer table grows an expand column; rows with many properties 
render a bounded nested table inside the expanded row, matching the consumer 
page's existing nested-table pattern.
   - Differences from the classic dialog: Studio renders properties inside the 
existing drawer as an expandable row instead of a separate modal, and applies 
the bounded payload contract the classic dialog never had.
   
   (head f77c10a2c4b9dfbc539f361b8405cea2eebabf04; verification details in the 
evidence comment below)
   


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

Reply via email to