unbridled-41 commented on PR #4871:
URL:
https://github.com/apache/rocketmq-dashboard/pull/4871#issuecomment-5929278491
Thanks for the review — all items are addressed in 005d7bb1 (plus a
PR-description update just pushed):
1. **Mirror test** —
`MybatisPlusAlertStateRepositoryTest#findsActiveStatesToleratesACorruptLabelsRowInsteadOfFailingTheQueryTest`
feeds the joined alert row a `{not-json` labels column and asserts the state
read still succeeds with empty labels. Reverting
`MybatisPlusAlertStateRepository.java:209` to `throw` makes it red.
2. **Warn with row identity** — both catch blocks now log
`log.warn("Degrading unreadable labels of system alert {} to empty labels: {}",
alertId, error.toString())` (the `AiEventCodec.read` style), with the alert id
threaded through the now-`readLabels(Long alertId, String labelsJson)`
signature in both repositories.
3. **Trigger provenance** — the PR description now has a "Where a corrupt
labels_json row can come from" section: the application's own write path always
serialises a `TreeMap`, so the reachable sources are manual DB edits on the
nullable TEXT column, an aborted migration or partial restore, and external
writers sharing the database.
4. **Asymmetry documented** — the description also states explicitly why
`MybatisPlusAlertSilenceRepository` and `MybatisPlusMetricSnapshotRepository`
keep throwing: their label maps feed silence matching and metric evaluation,
where an empty map would silently change a decision instead of just a display.
5. **Rename** — the alert-repository case is now
`...InsteadOfFailingTheQueryTest` (suffix only).
`MybatisPlusAlertRepositoryTest` + `MybatisPlusAlertStateRepositoryTest` +
`NativeAlertProcessorTest` + `AlertServiceTest` → 130/130 (was 129; the mirror
test is the addition), checkstyle clean.
--
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]
